Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1693)

Unified Diff: extensions/renderer/resources/messaging.js

Issue 1567423002: Ensure that sendMessage's callback is called. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Clarify intent Created 4 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/test/data/extensions/api_test/messaging/connect_crash/test.js ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: extensions/renderer/resources/messaging.js
diff --git a/extensions/renderer/resources/messaging.js b/extensions/renderer/resources/messaging.js
index a15497d23efac334c9073830fcdd47d3237ac543..ef562779ad847e2d29d780e231b1c313c195a9c8 100644
--- a/extensions/renderer/resources/messaging.js
+++ b/extensions/renderer/resources/messaging.js
@@ -335,9 +335,13 @@
return;
}
- // Ensure the callback exists for the older sendRequest API.
- if (!responseCallback)
- responseCallback = function() {};
+ function sendResponseAndClearCallback(response) {
+ // Save a reference so that we don't re-entrantly call responseCallback.
+ var sendResponse = responseCallback;
+ responseCallback = null;
+ sendResponse(response);
+ }
+
// Note: make sure to manually remove the onMessage/onDisconnect listeners
// that we added before destroying the Port, a workaround to a bug in Port
@@ -346,14 +350,27 @@
// http://crbug.com/320723 tracks a sustainable fix.
function disconnectListener() {
- // For onDisconnects, we only notify the callback if there was an error.
- if (chrome.runtime && chrome.runtime.lastError)
- responseCallback();
+ if (!responseCallback)
Devlin 2016/01/08 18:20:01 nit: Maybe add a super short comment. if (!respons
robwu 2016/01/08 18:22:44 Or the user did not supply a callback in the first
Devlin 2016/01/08 21:48:09 Fair enough.
+ return;
+
+ if (lastError.hasError(chrome)) {
+ sendResponseAndClearCallback();
+ } else {
+ lastError.set(
+ port.name, 'The message port closed before a reponse was received.',
+ null, chrome);
+ try {
+ sendResponseAndClearCallback();
+ } finally {
+ lastError.clear(chrome);
+ }
+ }
}
function messageListener(response) {
try {
- responseCallback(response);
+ if (responseCallback)
+ sendResponseAndClearCallback(response);
} finally {
port.disconnect();
}
« no previous file with comments | « chrome/test/data/extensions/api_test/messaging/connect_crash/test.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698