Enable JSBinding for WebMessageListener to be world-aware. [chromium/src : main]
Richard (Torne) Coles (Gerrit)
Richard (Torne) Coles voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Cathy Li
- Peter Birk Pakkenberg
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: aj...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): aj...@chromium.org
Reviewer source(s):
aj...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Alex Gough
- Peter Birk Pakkenberg
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Gough (Gerrit)
Alex Gough added 1 comment![]( "Open in Gerrit")
int32 js_world;does this have to be an int - can the identifier be a token or some other unordered thing? (ints can lead to out of bound indexing, or instance)
does this have to be signed? what do negative values mean?
Related details
- Cathy Li
- Peter Birk Pakkenberg
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Richard (Torne) Coles (Gerrit)
Richard (Torne) Coles added 1 comment![]( "Open in Gerrit")
int32 js_world;does this have to be an int - can the identifier be a token or some other unordered thing? (ints can lead to out of bound indexing, or instance)
does this have to be signed? what do negative values mean?
This is just wrapping various underlying APIs in //content and Blink that take a world ID, which is defined as int32/int32_t/etc in those places (e.g. `WebLocalFrame::GetScriptContextFromWorldId` or in the mojo interface [here](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/frame/frame.mojom;l=1119;drc=f4bfa8d7aeb32dd84e7f35e6ffbec47420c43e5d)).
For the actual feature being implemented here there is no specific meaning to the values here at all other than that 0 means the main world.
I don't know offhand how other features in Chromium that use the underlying isolated world support assign/check IDs. Cathy, do you know e.g. how extensions do this?
Alex Gough (Gerrit)
Alex Gough added 1 comment![]( "Open in Gerrit")
int32 js_world;Richard (Torne) Colesdoes this have to be an int - can the identifier be a token or some other unordered thing? (ints can lead to out of bound indexing, or instance)
does this have to be signed? what do negative values mean?
This is just wrapping various underlying APIs in //content and Blink that take a world ID, which is defined as int32/int32_t/etc in those places (e.g. `WebLocalFrame::GetScriptContextFromWorldId` or in the mojo interface [here](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/frame/frame.mojom;l=1119;drc=f4bfa8d7aeb32dd84e7f35e6ffbec47420c43e5d)).
For the actual feature being implemented here there is no specific meaning to the values here at all other than that 0 means the main world.
I don't know offhand how other features in Chromium that use the underlying isolated world support assign/check IDs. Cathy, do you know e.g. how extensions do this?
makes sense - can you add a comment that explains the type, then we're ok.
Cathy Li (Gerrit)
Cathy Li added 1 comment![]( "Open in Gerrit")
int32 js_world;Richard (Torne) Colesdoes this have to be an int - can the identifier be a token or some other unordered thing? (ints can lead to out of bound indexing, or instance)
does this have to be signed? what do negative values mean?
This is just wrapping various underlying APIs in //content and Blink that take a world ID, which is defined as int32/int32_t/etc in those places (e.g. `WebLocalFrame::GetScriptContextFromWorldId` or in the mojo interface [here](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/frame/frame.mojom;l=1119;drc=f4bfa8d7aeb32dd84e7f35e6ffbec47420c43e5d)).
For the actual feature being implemented here there is no specific meaning to the values here at all other than that 0 means the main world.
I don't know offhand how other features in Chromium that use the underlying isolated world support assign/check IDs. Cathy, do you know e.g. how extensions do this?
I'm not super sure how extensions enumerate isolated worlds - I assume there's an extension registry somewhere that assigns world IDs to the extension that requests it.
the underlying blink API is the DOMWrapperWorld here, where -1 means an invalid world (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/bindings/dom_wrapper_world.h;l=74;drc=a9f09a1411a697ace066a84efd018f49c28cc8a3)
Related details
- Alex Gough
- Peter Birk Pakkenberg
- Richard (Torne) Coles
Cathy Li (Gerrit)
Cathy Li added 1 comment![]( "Open in Gerrit")
Richard (Torne) Colesdoes this have to be an int - can the identifier be a token or some other unordered thing? (ints can lead to out of bound indexing, or instance)
does this have to be signed? what do negative values mean?
Cathy LiThis is just wrapping various underlying APIs in //content and Blink that take a world ID, which is defined as int32/int32_t/etc in those places (e.g. `WebLocalFrame::GetScriptContextFromWorldId` or in the mojo interface [here](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/public/mojom/frame/frame.mojom;l=1119;drc=f4bfa8d7aeb32dd84e7f35e6ffbec47420c43e5d)).
For the actual feature being implemented here there is no specific meaning to the values here at all other than that 0 means the main world.
I don't know offhand how other features in Chromium that use the underlying isolated world support assign/check IDs. Cathy, do you know e.g. how extensions do this?
I'm not super sure how extensions enumerate isolated worlds - I assume there's an extension registry somewhere that assigns world IDs to the extension that requests it.
the underlying blink API is the DOMWrapperWorld here, where -1 means an invalid world (https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/bindings/dom_wrapper_world.h;l=74;drc=a9f09a1411a697ace066a84efd018f49c28cc8a3)
Done
Related details
- Alex Gough
- Peter Birk Pakkenberg
- Richard (Torne) Coles
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Gough (Gerrit)
Alex Gough voted and added 1 comment![]( "Open in Gerrit")
- Cathy Li
- Peter Birk Pakkenberg
- Richard (Torne) Coles
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cathy Li (Gerrit)
Cathy Li voted Commit-Queue+2![]( "Open in Gerrit")
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Enable JSBinding for WebMessageListener to be world-aware.
Design doc:
https://docs.google.com/document/d/1WVLFAKlPIXGt-O8Xve1r7qLpeRY60xCJ7XIym8kEJwI/edit?tab=t.0#heading=h.7nki9mck5t64
In this change: introduce the ability for message listeners to be added
to isolated worlds: 0 meaning main world, X>0 meaning an isolated world
ID. MessageListeners are now keyed by (name, worldId) rather than simply
(name) as was previous.
- M android_webview/browser/aw_contents.cc
- M android_webview/browser/aw_contents.h
- M android_webview/java/src/org/chromium/android_webview/AwContents.java
- M components/js_injection/browser/js_communication_host.cc
- M components/js_injection/browser/js_communication_host.h
- M components/js_injection/browser/navigation_listener_browsertest.cc
- M components/js_injection/common/interfaces.mojom
- M components/js_injection/renderer/js_binding.cc
- M components/js_injection/renderer/js_binding.h
- M components/js_injection/renderer/js_communication.cc
Code-Review: +1 by Richard (Torne) Coles, +1 by Alex Gough
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |