New issue
Advanced search Search tips

Issue 891572 link

Starred by 1 user
Status: Assigned
Owner:
rdevlin....@chromium.org
Cc:
karandeepb@chromium.org
clamy@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Click to script: Updating wants-to-run badge UI can be racy in case of withheld web requests.

Project Member Reported by karandeepb@chromium.org, Oct 3 
Currently we clear the pending script injections/web requests for a web contents in ExtensionActionRunner::DidFinishNavigation.

While this works for the general case, this is not completely correct. 

[1] A main-frame navigation will be committed only after its network request is seen by the browser. Consider that a main-frame navigation request is the only withheld request for a page. ExtensionActionRunner::DidFinishNavigation will then clear |web_request_blocked_| once the main-frame navigation commits and we won't show the wants-to-run badging UI on the page.

[2] This may also be true for network requests for main-frame subresources that reach the browser before the navigation has committed. The record for any such withheld requests will be lost once the main-frame navigation commits. (While this is rare, it's possible).

The core issue here is that it's not trivial to map a network request to the correct navigation. (We can map it to a render frame but not the navigation).

Not sure if this might be an issue for script injection as well.

Comment 1 by rdevlin....@chromium.org, Oct 4

Cc: karandeepb@chromium.org
Interesting.  I agree this is a problem for the main frame navigation case.  Fixing that will be... fun.  I wonder if we'll need to do something like tracking navigation requests separately, and not clearing those on navigation if they're for the same origin that is being committed.

I'm not sure that this is an issue for subresources - is it possible for a subresource request to hit the browser process before navigation commits?  I thought we wouldn't being loading a page until after commit time.  Similarly for script injection, we don't inject scripts until after the document element is available, which should (I hope) be deterministically after commit.

Comment 2 by karandeepb@chromium.org, Oct 4 

>> I'm not sure that this is an issue for subresources - is it possible for a subresource request to hit the browser process before navigation commits?  
It is possible. IIRC I think something like this happens:

1. We start a request for the navigation url.
2. We get the response back. 
3. We choose the renderer and call ReadyToCommitNavigation.
4. We ask the renderer to commit the navigation.
5. We receive an IPC from the renderer and dispatch DidFinishNavigation.

B/w 2 and 5, we are already reading the response body for the navigation request which can lead to sub-resource requests on the IO thread.

>> Similarly for script injection, we don't inject scripts until after the document element is available, which should (I hope) be deterministically after commit.

I don't have much context into script injection to say for sure. But I'd look at how document_start works in the renderer.

Comment 3 by karandeepb@chromium.org, Oct 4

Cc: clamy@chromium.org
I faced similar problems for DNR whitelisting API and had implemented additional book-keeping. But the logic turned out to be quite complex.

I remember clamy@ mentioning that there were plans to add some sort of navigation_id to network requests to help with cases like these. cc'ing her.
►

Sign in to add a comment