New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 597698 link

Starred by 4 users

Issue metadata

Status: Verified
Owner:
Closed: Dec 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

sendMessage fails strangely

Project Member Reported by batz@chromium.org, Mar 24 2016

Issue description

Version: 50.0.2661.49 (also happens in M49)
OS: All

What steps will reproduce the problem?
(1) Add an onMessage listener to an extension
(2) call sendMessage after the extension is loaded (but before opening the options page)

Alternative Strangeness:
(1) Add an onMessage listener to an extension
(2) Open the options page to work around the above bug
(3) call sendMessage with a callback

What is the expected output?
In both cases the sendMessage should call the onMessage listener, and in the latter it should also be called back with success.

What do you see instead?
In both cases we observe:
Could not establish connection. Receiving end does not exist.
chrome.runtime.sendMessage({'action': 'test'});
in the lastError.message.

It's worth noting even if I generate a background.html by hand and use it instead of the script it still breaks.

I've attached a very stupid background, manifest, and options.html which reproduce this.  My Javascript console interaction:
init

> chrome.runtime.sendMessage({'action': 'test'});
undefined

# Here I'd expect the log to show the message sent, as it's printed in onMessage, see below

> chrome.runtime.sendMessage({'action': 'test'}, function(resp) { if (chrome.runtime.lastError) { console.log('ERR: ' + chrome.runtime.lastError.message); }});
undefined
ERR: Could not establish connection. Receiving end does not exist.

# Now I open the options page

> chrome.runtime.sendMessage({'action': 'test'}, function(resp) { if (chrome.runtime.lastError) { console.log('ERR: ' + chrome.runtime.lastError.message); }});
undefined
ERR: Could not establish connection. Receiving end does not exist.

> chrome.runtime.sendMessage({'action': 'test'});
undefined
background_js_compiled.js:10 onMessage: {"action":"test"}

# Those last two are especially confusing.

I believe this is a dupe of:
https://bugs.chromium.org/p/chromium/issues/detail?id=592478&q=sendMessage&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified
but I'm not 100% sure given the details in the other bug.

Please use labels and text to provide additional information.
 
background_js_compiled.js
324 bytes View Download
manifest.json
792 bytes View Download
options.html
93 bytes View Download

Comment 1 by t...@google.com, Mar 25 2016

Cc: t...@google.com
I looked at the bugs in the TE-Verified-M49. The error now shows up in M49 because of https://bugs.chromium.org/p/chromium/issues/detail?id=439780.


Comment 2 by bhp@google.com, Mar 25 2016

Cc: bhp@google.com

Comment 3 by rob@robwu.nl, Apr 6 2016

Components: Platform>Extensions
Labels: -Pri-3 M-52 Pri-2
Owner: rob@robwu.nl
Status: Assigned (was: Untriaged)
This is like  issue 479425 , except the fix does not work because messages to an extension are sent to the process instead of a specific frame, and the browser nor renderer filters the port (sendMessage is implemented as creating a port, sending a port (and closing the port if there is no callback)).

In resources/messaging.js, there is a last check that prevents the port from connecting if it is known. When a callback is passed to sendMessage, the port stays open so the port is known and no erroneous connections are created.
When a callback is not set, the port is disconnected and erased from the set of ports. As a result, the filter does not work and the extension is notified of the message.

I've attached a test case that shows the root cause of the issue.

Preferably, the port lifetime management logic should be in the browser process, but I also want to keep the optimization of sending one IPC to the extension process instead of notifying every frame separately, since extension messages are quite common, and especially with site isolation enabled, there may be many extension frames in a single process. Given this trade-off, I will probably explore the option of storing the RFH ID of the opener frame in the message, or maintain the port ID set in the renderer.
manifest.json
195 bytes View Download
background.js
994 bytes View Download
Project Member

Comment 4 by sheriffbot@chromium.org, Jun 1 2016

Labels: -M-52 M-53 MovedFrom-52
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by sheriffbot@chromium.org, Jul 14 2016

Labels: -M-53 MovedFrom-53
This issue has been moved once and is lower than Pri-1. Removing the milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by rob@robwu.nl, Aug 7 2016

Cc: rob@robwu.nl rnimmagadda@chromium.org
 Issue 628083  has been merged into this issue.
"Well that's just great, Rob. You changed the Chrome documentation but didn't changed the Chrome code to behave as per your revised documentation. You did though take the time to revise the Firefox code per your doc changes, so now Firefox and Chrome behave very differently and you've pissed off.

Can someone who has a stake in Chrome fix this?" ( Issue 628083  comment 14)

Ah, and then killed the bug report by merging it into this unrelated dead-end.

Comment 8 by rob@robwu.nl, Oct 28 2016

@natalie (#7)
In either case the documentation did not match the actual behavior, but now the documentation more accurately represents what developers should expect from the API.


> Ah, and then killed the bug report by merging it into this unrelated dead-end.

The root causes are identical, and hence merging the bug is the right thing to do.
Rest assured that this bug is eventually going to be fixed (thanks for reminding by the way, I kind of forgot about this issue).

Comment 9 by rob@robwu.nl, Nov 27 2016

Labels: M-57
Status: Started (was: Assigned)
Patch: https://codereview.chromium.org/2529213002
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 6 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8e744a465d70e2ab1bf7959a66c3c65bd3850092

commit 8e744a465d70e2ab1bf7959a66c3c65bd3850092
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Tue Dec 06 18:40:27 2016

[Extensions] Extension Port Ids and Initialization 2.0

Extensions can open message ports to communicate between different
contexts. A port can have n different receivers, and communication can
flow bidirectionally.

Currently, this is implemented by the port having a global id, vended by
the browser process, which uniquely identifies a port (amongst all
contexts). In making port creation asynchronous ( crbug.com/642380 ), we
also added the concept of a "local id" to be used in Javascript that
identified a port uniquely in the context. This allowed us to have the
browser asynchronously message us back a global id for the port to use.

Unfortunately, this still has a number of problems:
- Asynchronous port creation doesn't work in onunload
  ( crbug.com/660706 ); this also required exposing the unload state from
  blink.
- The time-to-message is slower, since we wait on an IPC round trip
  ( crbug.com/649942 ).

Instead, construct a new PortId concept as a combination of three
members: context id (a globally-unique id for the context), port number
(a port id within that context), and whether or not the port is an
opener. This allows us to:
- Make a port ID on the renderer, thus making it entirely synchronous
  and even faster than before (we don't need the browser involved at
  all). Also without exposing knowledge of unload state.
- Identify whether the opposite end of the port was created in the same
  context (solving the issue of re-opening extension ports in a same-
  process renderer,  crbug.com/597698 ). Test added courtesy of
  rob@robwu.nl.
- Increase difficulty in sending bad IPCs since an attacker would have
  to guess another context's GUID (rather than just being able to know
  that a port with id '1' exists somewhere).

Additionally, eliminate a few hundred lines of code.

BUG= 649942 
BUG= 597698 

Review-Url: https://codereview.chromium.org/2547753002
Cr-Commit-Position: refs/heads/master@{#436655}

[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/browser/extensions/api/messaging/extension_message_port.cc
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/browser/extensions/api/messaging/extension_message_port.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/browser/extensions/api/messaging/message_service.cc
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/browser/extensions/api/messaging/message_service.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/browser/extensions/api/messaging/native_message_port.cc
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/browser/extensions/api/messaging/native_message_port.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/browser/renderer_host/chrome_extension_message_filter.cc
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/browser/renderer_host/chrome_extension_message_filter.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/renderer/chrome_mock_render_thread.cc
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/renderer/chrome_mock_render_thread.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/chrome/test/data/extensions/api_test/messaging/background_only/test.js
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/common/BUILD.gn
[add] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/common/api/messaging/port_id.cc
[add] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/common/api/messaging/port_id.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/common/extension_messages.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/renderer/dispatcher.cc
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/renderer/dispatcher.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/renderer/extension_frame_helper.cc
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/renderer/extension_frame_helper.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/renderer/extension_port.cc
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/renderer/extension_port.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/renderer/messaging_bindings.cc
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/extensions/renderer/messaging_bindings.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/third_party/WebKit/Source/web/WebDocument.cpp
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/third_party/WebKit/public/web/WebDocument.h
[modify] https://crrev.com/8e744a465d70e2ab1bf7959a66c3c65bd3850092/tools/metrics/histograms/histograms.xml

Rob, anything else needed here?  Or can we close this out?

Comment 12 by rob@robwu.nl, Dec 19 2016

Status: Verified (was: Started)
This issue has been fixed.

In Chrome 55.0.2883.75, running the extension from comment 3 results in the following message in the console of the background page:
Unexpected port: after tab disco

In Chrome ​57.0.2957.0, doing the same does not yield any message, as expected.

Comment 13 by woxxom@gmail.com, Dec 31 2016

Now runtime.onMessage listener in the same page of runtime.sendMessage is not getting triggered, see  issue 677692 .

Sign in to add a comment