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

Side by Side 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 unified diff | 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 »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 // chrome.runtime.messaging API implementation. 5 // chrome.runtime.messaging API implementation.
6 6
7 // TODO(kalman): factor requiring chrome out of here. 7 // TODO(kalman): factor requiring chrome out of here.
8 var chrome = requireNative('chrome').GetChrome(); 8 var chrome = requireNative('chrome').GetChrome();
9 var Event = require('event_bindings').Event; 9 var Event = require('event_bindings').Event;
10 var lastError = require('lastError'); 10 var lastError = require('lastError');
(...skipping 317 matching lines...) Expand 10 before | Expand all | Expand 10 after
328 328
329 if (port.name == kMessageChannel && !responseCallback) { 329 if (port.name == kMessageChannel && !responseCallback) {
330 // TODO(mpcomplete): Do this for the old sendRequest API too, after 330 // TODO(mpcomplete): Do this for the old sendRequest API too, after
331 // verifying it doesn't break anything. 331 // verifying it doesn't break anything.
332 // Go ahead and disconnect immediately if the sender is not expecting 332 // Go ahead and disconnect immediately if the sender is not expecting
333 // a response. 333 // a response.
334 port.disconnect(); 334 port.disconnect();
335 return; 335 return;
336 } 336 }
337 337
338 // Ensure the callback exists for the older sendRequest API. 338 function sendResponseAndClearCallback(response) {
339 if (!responseCallback) 339 // Save a reference so that we don't re-entrantly call responseCallback.
340 responseCallback = function() {}; 340 var sendResponse = responseCallback;
341 responseCallback = null;
342 sendResponse(response);
343 }
344
341 345
342 // Note: make sure to manually remove the onMessage/onDisconnect listeners 346 // Note: make sure to manually remove the onMessage/onDisconnect listeners
343 // that we added before destroying the Port, a workaround to a bug in Port 347 // that we added before destroying the Port, a workaround to a bug in Port
344 // where any onMessage/onDisconnect listeners added but not removed will 348 // where any onMessage/onDisconnect listeners added but not removed will
345 // be leaked when the Port is destroyed. 349 // be leaked when the Port is destroyed.
346 // http://crbug.com/320723 tracks a sustainable fix. 350 // http://crbug.com/320723 tracks a sustainable fix.
347 351
348 function disconnectListener() { 352 function disconnectListener() {
349 // For onDisconnects, we only notify the callback if there was an error. 353 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.
350 if (chrome.runtime && chrome.runtime.lastError) 354 return;
351 responseCallback(); 355
356 if (lastError.hasError(chrome)) {
357 sendResponseAndClearCallback();
358 } else {
359 lastError.set(
360 port.name, 'The message port closed before a reponse was received.',
361 null, chrome);
362 try {
363 sendResponseAndClearCallback();
364 } finally {
365 lastError.clear(chrome);
366 }
367 }
352 } 368 }
353 369
354 function messageListener(response) { 370 function messageListener(response) {
355 try { 371 try {
356 responseCallback(response); 372 if (responseCallback)
373 sendResponseAndClearCallback(response);
357 } finally { 374 } finally {
358 port.disconnect(); 375 port.disconnect();
359 } 376 }
360 } 377 }
361 378
362 privates(port).impl.onDestroy_ = function() { 379 privates(port).impl.onDestroy_ = function() {
363 port.onDisconnect.removeListener(disconnectListener); 380 port.onDisconnect.removeListener(disconnectListener);
364 port.onMessage.removeListener(messageListener); 381 port.onMessage.removeListener(messageListener);
365 }; 382 };
366 port.onDisconnect.addListener(disconnectListener); 383 port.onDisconnect.addListener(disconnectListener);
(...skipping 26 matching lines...) Expand all
393 exports.$set('Port', Port); 410 exports.$set('Port', Port);
394 exports.$set('createPort', createPort); 411 exports.$set('createPort', createPort);
395 exports.$set('sendMessageImpl', sendMessageImpl); 412 exports.$set('sendMessageImpl', sendMessageImpl);
396 exports.$set('sendMessageUpdateArguments', sendMessageUpdateArguments); 413 exports.$set('sendMessageUpdateArguments', sendMessageUpdateArguments);
397 414
398 // For C++ code to call. 415 // For C++ code to call.
399 exports.$set('hasPort', hasPort); 416 exports.$set('hasPort', hasPort);
400 exports.$set('dispatchOnConnect', dispatchOnConnect); 417 exports.$set('dispatchOnConnect', dispatchOnConnect);
401 exports.$set('dispatchOnDisconnect', dispatchOnDisconnect); 418 exports.$set('dispatchOnDisconnect', dispatchOnDisconnect);
402 exports.$set('dispatchOnMessage', dispatchOnMessage); 419 exports.$set('dispatchOnMessage', dispatchOnMessage);
OLDNEW
« 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