New issue
Advanced search Search tips

Issue 828664 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: 2018-04-16
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

runtime.sendMessage gives bogus error in Canry

Project Member Reported by jamiewa...@chromium.org, Apr 3 2018

Issue description

Chrome Version: 67.0.3386.0
OS: Mac

What steps will reproduce the problem?
(1) Run a Chrome app (extension might also work; I used Chrome Remote Desktop).
(2) Open the console.
(3) Run "chrome.runtime.sendMessage(null, 'test')"

What is the expected result?
No error.

What happens instead?
An error:

Uncaught TypeError: Error in invocation of runtime.sendMessage(optional string extensionId, any message, optional object options, optional function responseCallback): Missing required argument 'message'.

 
Labels: -ReleaseBlock-Beta Target-67 FoundIn-67 OS-Linux OS-Windows
Able to reproduce the issue on latest stable-65.0.3325.181, beta-66.0.3359.81, dev-67.0.3386.1 & Canary-67.0.3394.0 as per C#0 on Mac 10.13.3, Debian Rodate & Windows 10.

Observed below error when we run the command-"chrome.runtime.sendMessage(null, 'test')" in console

extensions::runtime:71 Uncaught Error: chrome.runtime.connect() called from a webpage must specify an Extension ID (string) for its first argument
    at Object.<anonymous> (VM27 extensions::runtime:71)
    at Object.handleRequest (VM18 extensions::binding:64)
    at Object.<anonymous> (VM18 extensions::binding:374)
    at Object.<anonymous> (VM27 extensions::runtime:56)
    at Object.handleRequest (VM18 extensions::binding:64)
    at Object.<anonymous> (VM18 extensions::binding:374)
    at <anonymous>:1:16

Same issue observed from M60 builds to latest Canary hence removing blocker label.Could someone from dev team please take a look into it.
Please find the attached screencast for reference.

Thanks..!


828664.PNG
109 KB View Download
Labels: ReleaseBlock-Beta
That is expected behaviour for a web page (read the error messsage carefully), but apps and extensions do not need to specify an extension id. The regression is that the second "message" argument, which is mandatory is not being marshalled correctly.

Reinstating the RB tag, as I think it's a regression.

Comment 3 by gov...@chromium.org, Apr 11 2018

A friendly reminder that M67 branch is tomorrow, Thursday 04/12! Your bug is labelled as Beta ReleaseBlock, pls make sure to land the fix ASAP to trunk. This way we branch M67 from a high quality trunk. Thank you.
Components: -Blink>JavaScript Blink
Not V8 related.
Components: -Blink Platform>Extensions

Comment 6 by gov...@chromium.org, Apr 12 2018

Any update on this bug as M67 is branching today (04/12) and this bug is marked as M67 Beta Blocker?
Labels: -ReleaseBlock-Beta -Pri-2 ReleaseBlock-Stable Pri-1
Owner: rdevlin....@chromium.org
Status: Assigned (was: Untriaged)
This need not block beta.

I'll get a fix out soon.
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 13 2018

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

commit 7c723eb6a8985d10a352469e1231eb17a021a8e1
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri Apr 13 19:38:26 2018

[Extensions Bindings] Tweak messaging argument parsing

There was a bug where if the ID were passed as null or undefined (rather
than completely omitted), the message would fail to parse. Fix this, and
add a unittest.

Bug:  828664 

Change-Id: I92b0f071a1627322e12e8eaf0f5787ccd0793138
Reviewed-on: https://chromium-review.googlesource.com/1012601
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550718}
[modify] https://crrev.com/7c723eb6a8985d10a352469e1231eb17a021a8e1/extensions/renderer/messaging_util.cc
[modify] https://crrev.com/7c723eb6a8985d10a352469e1231eb17a021a8e1/extensions/renderer/runtime_hooks_delegate_unittest.cc

Labels: OS-Chrome
NextAction: 2018-04-16
I think this should be fixed with #8.

jamiewalch@, can you double-check in Canary on Monday?
The NextAction date has arrived: 2018-04-16
Labels: Merge-Request-67
Status: Verified (was: Assigned)
Confirmed fixed in 68.0.3397.0; thanks!
Awesome, thanks jamiewalch@!
Project Member

Comment 13 by sheriffbot@chromium.org, Apr 17 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Pls merge your change to M67 branch 3396 by 2:00 PM PT today, Tuesday so we can pick it up for tomorrow's M67 dev release. Thank you.
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 17 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aeff192f2c89f2465b0b53b7c3b7d4b8436b577e

commit aeff192f2c89f2465b0b53b7c3b7d4b8436b577e
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Tue Apr 17 17:49:48 2018

[Merge m67][Extensions Bindings] Tweak messaging argument parsing

There was a bug where if the ID were passed as null or undefined (rather
than completely omitted), the message would fail to parse. Fix this, and
add a unittest.

Bug:  828664 

TBR=rdevlin.cronin@chromium.org

(cherry picked from commit 7c723eb6a8985d10a352469e1231eb17a021a8e1)

Change-Id: I92b0f071a1627322e12e8eaf0f5787ccd0793138
Reviewed-on: https://chromium-review.googlesource.com/1012601
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550718}
Reviewed-on: https://chromium-review.googlesource.com/1015412
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#50}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/aeff192f2c89f2465b0b53b7c3b7d4b8436b577e/extensions/renderer/messaging_util.cc
[modify] https://crrev.com/aeff192f2c89f2465b0b53b7c3b7d4b8436b577e/extensions/renderer/runtime_hooks_delegate_unittest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c723eb6a8985d10a352469e1231eb17a021a8e1

commit 7c723eb6a8985d10a352469e1231eb17a021a8e1
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date: Fri Apr 13 19:38:26 2018

[Extensions Bindings] Tweak messaging argument parsing

There was a bug where if the ID were passed as null or undefined (rather
than completely omitted), the message would fail to parse. Fix this, and
add a unittest.

Bug:  828664 

Change-Id: I92b0f071a1627322e12e8eaf0f5787ccd0793138
Reviewed-on: https://chromium-review.googlesource.com/1012601
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550718}
[modify] https://crrev.com/7c723eb6a8985d10a352469e1231eb17a021a8e1/extensions/renderer/messaging_util.cc
[modify] https://crrev.com/7c723eb6a8985d10a352469e1231eb17a021a8e1/extensions/renderer/runtime_hooks_delegate_unittest.cc

Sign in to add a comment