New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track the origin of webRequest events #10429

Merged
merged 7 commits into from Sep 13, 2017

Conversation

Projects
None yet
3 participants
@qazbnm456
Contributor

qazbnm456 commented Sep 2, 2017

This PR is a mitigation to #10061, and therefore ppl can use the webContentsGetId to track the origin requester. The variable name webContentsGetId is taken from a64978b.

In addition, it would be convenient if we can have a webContents.fromGetId api to get the corresponding WebContents, just like what webContents.fromId does. How do you think?

@MarshallOfSound

Please remove package-lock.json 👍

@qazbnm456

This comment has been minimized.

Contributor

qazbnm456 commented Sep 2, 2017

@MarshallOfSound Done 😃

if (info) {
int64_t process_id = info->GetChildID();
int64_t routing_id = info->GetRouteID();
details->SetDouble("webContentsGetId", (process_id << 32) + routing_id);

This comment has been minimized.

@MarshallOfSound

MarshallOfSound Sep 2, 2017

Member

This should probably be just webContentsId

And assuming my understanding of this bit shifting is correct that ID should just work with https://electron.atom.io/docs/api/web-contents/#webcontentsfromidid 👍

This comment has been minimized.

@qazbnm456

qazbnm456 Sep 3, 2017

Contributor
  • webContentsId is actually the ID in a weak map, as shown here.
  • webContentsGetId is, in the other hand, a 64-bit integer composed of both the process id(high 32) and the RenderViewHost routing id(low 32), as shown here.

Thus, the argument passed to webContents.fromId(id) should be webContentsId, not webContentsGetId, as shown here. Besides, there's no api to get back the corresponding webContents from webContentsGetId afaik 😭 .

This comment has been minimized.

@zcbenz

zcbenz Sep 12, 2017

Contributor

I think you can use the process ID and routing ID to get the WebContents with:

content::WebContents* webContents = content::WebContents::FromRenderFrameHost(RenderFrameHost::FromID(process_id, routing_id));

And then get the webContentsId by calling api::WebContents::GetIDFromWrappedClass(webContents).

(We would need to make that method public though).

@qazbnm456 qazbnm456 changed the title from Track the origin requesters of webRequest events to Track the origin of webRequest events Sep 5, 2017

if (info) {
int64_t process_id = info->GetChildID();
int64_t routing_id = info->GetRouteID();
details->SetDouble("webContentsGetId", (process_id << 32) + routing_id);

This comment has been minimized.

@zcbenz

zcbenz Sep 12, 2017

Contributor

I think you can use the process ID and routing ID to get the WebContents with:

content::WebContents* webContents = content::WebContents::FromRenderFrameHost(RenderFrameHost::FromID(process_id, routing_id));

And then get the webContentsId by calling api::WebContents::GetIDFromWrappedClass(webContents).

(We would need to make that method public though).

@qazbnm456

This comment has been minimized.

Contributor

qazbnm456 commented Sep 12, 2017

@zcbenz
Thanks for your suggestions. I've used process_id and render_id, which is fetched from info-> GetRenderFrameID(), to get the corresponding webContents through content::WebContents::FromRenderFrameHost(content::RenderFrameHost::FromID(process_id, render_id)), and then get the webContentsId by atom::api::WebContents::GetIDFromWrappedClass(webContents). 😄

By the way, I just changed atom::api::WebContents::GetIDFromWrappedClass(webContents) from protected to public method.

@zcbenz

zcbenz approved these changes Sep 13, 2017

@zcbenz zcbenz merged commit 2048a1a into electron:master Sep 13, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@qazbnm456 qazbnm456 deleted the qazbnm456:web-request-origin branch Sep 13, 2017

qazbnm456 added a commit to LulumiProject/lulumi-browser that referenced this pull request Sep 29, 2017

1. update dependencies
2. remove mappings member in vuex
3. we now track each webRequest events by electron/electron#10429
4. store the corresponding tabId as an attribute into its webContents object

qazbnm456 added a commit to LulumiProject/lulumi-browser that referenced this pull request Sep 30, 2017

1. update dependencies
2. remove mappings member in vuex
3. we now track each webRequest events by electron/electron#10429
4. store the corresponding webContentsId as an attribute into extension API's TabObject
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment