Unable to reproduce the issue on Win-10 using chrome reported version #60.0.3112.101 and latest canary #62.0.3188.0.

Attached a screen cast for reference.

Following are the steps followed to reproduce the issue.
------------
1. Ran index.html
2. Then clicked on the "press" button.
3. Observed that an alert with window object appeared as expected.

anthony@ - Could you please verify the screen cast and please let us know if anything missed from our side.

Thanks...!!

Deleted: 755597.mp4 711 KB

	755597.mp4 711 KB View Download

I couldn't reproduce this with Google Chrome 60 and 62, local file and via a web server.

> UserAgent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.78 Safari/537.36 OPR/47.0.2631.39

Are you suing Opera?  Can you check the behavior with Google Chrome?

I created the bug from Opera.

The bug is reproduced time to time. When you see a scroll in iframe that mean bug is reproduced. I attached the video for Chrome beta 61.0.3163.49

Deleted: 2017-08-18 at 11-48-41.mp4 5.0 MB

	2017-08-18 at 11-48-41.mp4 5.0 MB View Download

Thank you for providing more feedback. Adding requester "krajshree@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Do you see this behavior via web server?

With file:///, the following behavior is expected:
 - If newSearchIFrame.contentDocument is accessed before finishing 'iframe.html' load, it doesn't throw because newSearchIFrame is about:blank created by the parent window.
 - If newSearchIFrame.contentDocument is accessed after finishing 'iframe.html' load, it throws because each of file resources is handled as unique origin.

I don't get error in via web server but the alert show parent as null, but it should be window.

Attached the video the bug via web server

Deleted: 2017-08-22 at 13-23-47.mp4 6.3 MB

	2017-08-22 at 13-23-47.mp4 6.3 MB View Download

Thank you for providing more feedback. Adding requester "tkent@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

> I don't get error in via web server but the alert show parent as null, but it should be window.

Thank you for the confirmation.  I also checked the behavior.

* If the browser finishes to load iframe.html before doc.write() overwrites the content of iframe.html, and alert(parent) shows the parent window expectedly.

* If the browser finishes to load iframe.html after doc.write(), iframe.html overwrites the document written by doc.write(). The document is detached from the IFRAME, and parent is null.

IMO, the current behavior is not wrong. However we should check if we changed the behavior intentionally since Google Chrome 59.

Oh, I couldn't reproduce the latter behavior with Chromium builds, and 60 stable after restarting the browser.  PlzNavigate issue?

Also, Firefox and Safari don't show the latter behavior. doc.write() should cancel on-going load request for iframe.html?

clamy@, can you take a look?  I know you looked at a navigation race recently for PlzNavigate, but document.write may be a different type of case.

It looks like on trunk, this works with and without plznavigate. on m61 (61.0.3163.59), it's only broken with plznavigate.

So I tried on trunk and it doesn't work with Plznavigate while it works with PlzNavigate disabled. I'm investigating.

So after investigating, this only happens if the iframe loads quickly in PlzNavigate.

When the iframe is slow, the following happens:
1) The browser process gets the iframe navigation request.
2) The renderer process does a document.write and sends an AbortNavigation IPC.
3) The browser process gets the AbortNavigation IPC and cancels the ongoing iframe navigation.
4) The iframe ends up with the document from the document write.

When the iframe is quick, the follwoing happens:
1) The browser process gets the iframe navigation request.
2) The renderer process does a document.write and sends an AbortNavigation IPC.
3) The iframe navigation is ready to commit, so the browser process sends a CommitIPC.
4) The browser process gets the AbortNavigation IPC but there is no navigation to abort since it was sent to the renderer for commit.
5) The renderer processes the CommitNavigation IPC and navigates the iframe. The iframe ends up with the data from the navigation.

I'm a bit surprised we get the iframe navigation start while executing JS, I thought the blink navigation scheduler prevented that.

Apart from that, this is a race condition between IPCs and I don't see how it's fixable. I think eventually we should have a sort of Mojo loader interface between renderers and the browser process (like we have for URLloaders), and with this the race could be fixed. With the IPC system though, it's no possible.

@tkent: how problematic is the change of behavior?

clamy@: This sounds similar to a race we had in session history navigations.  We fixed it using the interrupted_by_client_redirect check in RenderFrameImpl::NavigateInternal, which would cancel the history navigation at the equivalent of step 5 in your quick iframe case (comment 15).

However, I think I agree this would be harder to fix in PlzNavigate, since we're later in the navigation flow.  Before, the fix was about preventing the renderer from starting the navigation to the history item, but now it's about ignoring the request to commit the new document, which is basically too late.

It does seem like this is fundamentally a race between the iframe navigation and the subsequent document.write.  PlzNavigate just does a better job starting the navigation as soon as possible, so there's less of a chance to "undo" it like before.  I'm not sure whether that's something that needs fixing.

+japhet since we were just talking about NavigationScheduler on Friday.

+japhet and I were thinking that maybe we could get rid of NavigationScheduler in a PlzNavigate world, but examples like this might suggest that we should keep it around in some form for better determinism. The reason that this load isn't deferred is because we call out directly to the loader when creating an iframe:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.cpp?rcl=8cdee11d699afb9f8807aeb7faab2c619540b031&l=309

(Another example where navigations aren't currently deferred are link navigations...)

dcheng@: Interesting.  So if we used NavigationScheduler, we'd delay the point that we ask the browser process until the renderer finishes its JS (which might include canceling the navigation via document.write)?  I could see that being a reasonable approach.  As you mentioned offline, we might want to confirm that doesn't delay the navigation too much from a performance perspective, but I'm guessing it would usually be fine.

Using the NavigationScheduler is the equivalent of PostTask, so the delay will depend on the size of the message loop event queue. It will be hard to determine whether this is a big hit to performance or not without actual experimentation. If it is possible to implement it as "do it at the end of this iteration of the event loop", then we minimize the delay and the potential perf hit.

I do like having consistency in code, so I'd be very much interested in seeing us move over to using NavigationScheduler uniformly.

I'm not sure if the spec permits it, but using microtasks [1] would reduce the latency. However, we'd have to check if this behavior is permitted by the spec.

[1] https://html.spec.whatwg.org/multipage/webappapis.html#microtask-queue

IIUC, the micro-task will run before other tasks on the main message loop, but after its parent. That's probably what we want here.

@anthony@scand.com can you share more information on how you found this bug? i.e. are you running a webserver on the same machine? Or is it an intranet page?

I can't repro this when it's running off a webserver that's not on the same machine, because the delays in networking mean the first case in comment 15 is always taken.

So I have started looking at scheduling the iframe.src navigation, and I now have a layout_test version of the race (with the help of a busy loop in the middle) at https://chromium-review.googlesource.com/c/chromium/src/+/643287.

So about scheduling all kinds of navigation, at least for iframe creation it's turning to be a problem as it completely breaks the loading state of the page. Basically a lot of tests expect the iframe to be set in a loading state right away, which doesn't happen if the frame navigation is only scheduled :(. So if you have a page of the form:

<body>
stuff...
<iframe src="http://foo.com"></iframe>
</body>

we will report that we finished loading before starting to navigate to the iframe because the navigation was scheduled and did not start right away. Let's just say that this is breaking a lot of tests.

Of course we could modify the loading state by saying that if we scheduled a navigation the frame is loading, though it's likely to cause other issues (like what happens when the scheduled navigation is cancelled?).

We could also restrict the scheduling to iframe src assignation done through JavaScript, but I'm afraid we might have similar problems. For example:
<body>
stuff...
<script>
var iframe = document.createElement('iframe');
iframe.src = "http://foo.com";
document.body.appendChild(iframe);
</script>
</body>

is likely to report it finished loading before navigating the newly created iframe.

Beyond the scheduling issue, we're also having these issues because we have a lack of context between the different IPCs involved in the navigation. For example, we cannot link the CommitNavigation in this case with the DropNavigation IPC we sent to the browser before. In subresource loads we don't have this kind of issues because we know which load a resource_msg IPC is related to.

So one solution I have been thinking about is to do like resource loads and have an explicit mojo service (or a class) that the renderer uses when it wants the browser to navigate for it. For example, let's call them NavigationLoader and NavigationLoaderClient.

The RenderFrame holds a NavigationLoaderClient, and as long as this object exists it means the RenderFrame expects the browser process to do a navigation for it. On the other side, the browser process has a NavigationLoader (I would think owned by NavigationRequest).

The interface for these objects would be very simple:
- for NavigationLoader, Start and Cancel (Cancel would be called when the renderer no longer wants the navigation to happen, for example when there is a document.write).
- for NavigationLoaderClient, Cancelled, Commit and Failed (called respectively when the renderer should no longer expect a navigation or when the navigation is done - either successful or not).

In particular, it would not be possible for a navigation that originated from a NavigationLoaderClient to commit if its client is no longer there (provided it's committing in the same process - we should detach the navigation from the client when it switches processes). So, in the document.write case, we would just ask the frame to delete its NavigationLoaderClient (like we do with the pending DocumentLoader), which would notify the NavigationLoader to cancel, and prevent the navigation form committing if it was ready to commit.

Wdyt about it? Note that this modifying some PlzNavigate only code, so I'm thinking this is doable right now if we want to go in this direction.

I think it might be better to fix the navigation scheduling first, and then migrate to a Mojo interface to scope the navigations to a context? I'm worried about having queueing at a lot of different layers making things hard to reason about.

For navigation scheduler, it seems reasonable to me that having a queued navigation means that we're loading, while cancelling a queued navigation would finish loading: it doesn't seem different from, say, a slow navigation.

@dcheng: yes I agree that it makes sense to say that the frame is loading if a navigation is scheduled, the frame is loading. When we cancel it though, I think we have to check if the frame is not still loading (e.g. the navigation was scheduled while the frame was loading a page).

I looked at doing and this is where things get complicated. It seems that the ProgressTracker is really meant to be called from the FrameLoader. If we combine that with the issue I mentioned above about what to do when we unschedule the navigation, I was wondering why the NavigationScheduler is not part of the FrameLoader? To me it would seem simpler if every caller just called the FrameLoader Load method directly, and when we determine we're navigating cross-document we would call ScheduleLoad instead of StartLoad. ScheduleLoad would eventually call StartLoad when the scheduled load executes. I don't really see why we have the scheduler as a separate class with specific methods for every type of navigation we want to schedule (besides the legacy codebase).

Anyway, since I'm not very familiar with the NavigationScheduler, I'm not able to make progress quickly. Since I have 2 weeks left of work before being mostly OOO for a month (BlinkOn + vacation), I was wondering if dcheng@ or japhet@ could take a look at the issue? Thanks!