Pluming through navigation initiator's origin and url from blink to NavigationHandle |
|||||||||||||||
Issue descriptionPer our discussion, we want to know origin and url of the true navigation initiator, preferably make them available in NavigationHandle. This information will help us accurately identify the initiator where navigation is targeting another already exiting frames/tab.
,
Sep 30 2016
,
Oct 4 2016
,
Oct 7 2016
Assigning to dcheng for first step.
,
Oct 11 2016
,
Oct 11 2016
Just so it helps with prioritization: Deprecation of data: navigations is also blocked on getting this information. Intervention owners are asking if there is an estimated timeline for this work :)
,
Oct 19 2016
,
Nov 1 2016
dcheng@: Any updates here?
,
Nov 1 2016
,
Nov 1 2016
This should be P1 since it's blocking two P1's now.
,
Nov 1 2016
Sorry, forgot to update the status on this bug. I sent out an initial CL and talked with mkst@ 2 weeks ago to figure out the right plumbing for this (and to make sure it's not too redundant with what we already sort-of have). The current plan is to expose WebDocument as the requestingDocument on WebURLRequest. In didCommitProvisionalLoad(), we'll pull the original WebURLRequest and send the requesting origin and URL up to the browser then. WRT timeline, I'm currently finishing off another CL, but this is my priority for this week.
,
Nov 1 2016
Awesome, thanks for the update!
,
Dec 29 2016
It seems crrev.com/2416523002 is ready to land. Is there anything still blocking this CL?
,
Dec 29 2016
Unfortunately, it doesn't work well with OOPIF, so we need to do this tracking entirely browser-side... but then we need to worry about potential races. This is something I'll be chatting with nasko@ about to figure out a good resolution. Sorry for the delay.
,
Jan 10 2017
Hey @dcheng, any update? This change is critical so that we can block navigations to data: URIs. Please let us know if we can help unblock!
,
Jan 17 2017
Sorry for the delay! So just for documentation on the bug: At a high-level, we want to be able to track this on the NavigationHandle for a request. To do that, I originally wrote a CL to expose this as a Blink public API. The idea was that when a load commits, the renderer can pull the initiator URL out of the WebDataSource and send it up in an IPC to the browser process. Unfortunately, this doesn't work well for things like OOPIF: if a frame swaps processes, then the process that initiated the URL has to know to send the initiator URL up to the browser when it's swapped out... but because it's in a separate process, other data races are possible (the initiating frame could have been navigated elsewhere, for example). Another possibility is to always send the initiator URL when creating the NavigationHandle in the browser process, but that feels heavyweight and requires trusting the renderer. Instead, we'd like to just have NavigationHandle record the initiator's last committed URL when a NavigationHandle is created. This is a bit heavyweight (similar to how always sending the initiator URL from the renderer is), but it's necessary to avoid other sorts of races. However, I believe there are other races will prevent the initiator from being recorded correctly: 1. RenderFrame |initiator| initiates a navigation. 2. Before the browser process receives the navigation IPC, it sends an IPC to the renderer to swap out / detach |initiator|. 3. Browser process tries to lookup the initiator RFH, but it's gone (if it's a swap, I think it only stays in the pending delete list for 1 second; if it's a detach or the window is closed, it's removed immediately). Thus, we fail to record the initiator information. It's not clear to me how easy it is for a renderer to manipulate IPC timing to trigger this case... Finally, in the future, we believe we can make this lighterweight: creis@ is thinking about making RFH hold a pointer to the current FrameNavigationEntry (which would replace the last committed URL), making it lighterweight to record the initiator info. However, this still does not solve the IPC race issue...
,
Jan 17 2017
OK, after discussing this with alexmos, creis, and nasko, we're comfortable with just dropping the navigation IPC if the initiator lookup fails. That solves the potential IPC races =)
,
Mar 20 2017
I ran into this once again while implementing the blocking of data URLs. The blocking is done with a NavigationThrottle, and in order to support data to data navigations (e.g. links in data URLs) we need to know the initiator of the navigation. Looks like https://codereview.chromium.org/2416523002/ got the lgtm, any updates since then? :)
,
Mar 21 2017
,
Mar 30 2017
OK, so I'm finding that there's no satisfactory approach for implementing this. I've considered several approaches so far. Solution #1: https://codereview.chromium.org/2416523002/: plumb the info out from the renderer through WebDataSource. Originally, I intended this data to only be read at commit time: however, this is difficult due to how OOPIF works... if we force a process swap, it's likely that we can lose this data. Solution #2: https://codereview.chromium.org/2781723007/: this was the Blink side of the alternate approach to plumb the info out when beginning a navigation. The idea is once we know the initiator frame, we can send the frame routing ID up to the browser, and the browser uses this routing ID to fill in the origin/url of the initiator frame on the NavigationHandle. We're not trusting the renderer so send us the correct info, so yay. At least two problems... - navigations are typically scheduled. So if a frame schedules a navigation on another frame, and then detaches itself, we'll lose the initiator frame info (as Document::frame() is null when the Document is detached). - in case of IPC races to the browser (e.g. the initiator RFH gets swapped out while we're still sending the IPC), it's possible that we'll no longer be able to look up the initiator RFH. - probably others Solution #3: plumb navigations via their initiator frame. This is a large rewrite of how navigation works, and is likely impractical at the moment, since PlzNavigate has not yet shipped. The basic idea is that any navigation requests would be plumbed out via the initiator's WebFrameClient, along with the target frame... but this would make FrameLoader fraught with peril (since you must very carefully distinguish between |this| and |targetFrame|). There may be web compat issues as well: it's not clear if there's web content that expects to fire off navigations that still work after the source Document is detached. Solution #4: Use solution #1, but just always pass the info up. Non-ideal given that we have to trust the renderer again...
,
Mar 30 2017
Thanks for the update! It sounds like solution #2 is problematic because it tracks RFHs that can be detached. In my case (data URL blocking), I'm interested in knowing the top frame URL of the opener at the time the navigation was scheduled, mainly for window.open cases. Would it be possible to plumb out only the URL instead of the frame routing id? Does that solve the problem with the frame detaching itself, or having to look up the RFH? (obviously this only helps with my use case and not necessarily others :)
,
Mar 30 2017
I talked with Nasko to get content OWNERS signoff. There is no feasible way to implement a trustworthy initiator today: due to various races, we can't just plumb through a frame routing ID and hope to extract the correct information on the browser side. Instead, I'll land a version of the original CL to expose this information from Blink, and we'll pass the origin/URL up in RenderFrameImpl::didStartProvisionalLoad() / RenderFrameProxy::navigate(). It'll be bound to the NavigationHandle when it's created, and will be marked 'untrustworthy' (since the information comes from the renderer), but it should be sufficient to satisfy the current use cases we have. It's expected that we might be missing initiator information in certain contexts, but we can iterate from there and see what's still missing.
,
Mar 30 2017
Would the origin/URL be of the initiator frame? If so, would it be possible to also pass through the top frame URL of the initiator? In my use case, I want to allow data URL navigations if they are initiated by an iframe whose top frame is also a data URL. This is so that frames on data URLs can navigate to other data URLs. Doing that requires checking the top frame URL of the initiator instead of the frame for me (but I think jialul@ is interested in the actual initiator frame itself, so we'll need both?).
,
Mar 30 2017
We basically don't have this information for OOPIFs. Even if we did have it, I'm a bit hesistant to plumb out the initiator's top frame's URL as well--that's a lot of additional state on NavgationHandle. =(
,
Mar 30 2017
For safe browsing purpose, I'm OK with only knowing the top frame URL of the initiator too, if that makes things easier.
,
Mar 30 2017
Per comment 25, it makes things harder =(
,
May 1 2017
,
Jun 2 2017
,
Nov 10 2017
,
Dec 6 2017
Per comment #25, and since this issue no longer block 632514. Mark it as won'tfix. |
|||||||||||||||
►
Sign in to add a comment |
|||||||||||||||
Comment 1 by jialiul@chromium.org
, Sep 30 2016