PlzNavigate: properly execute BeforeUnload on renderer initiated navigations

components/test_runner/test_runner_for_specific_view.cc

Index: components/test_runner/test_runner_for_specific_view.cc
diff --git a/components/test_runner/test_runner_for_specific_view.cc b/components/test_runner/test_runner_for_specific_view.cc
index 4476f3d210e0e0d190601966a61653a2b544098c..482a9f583a93968a84eb7c7927c0c230a7157a60 100644
--- a/components/test_runner/test_runner_for_specific_view.cc
+++ b/components/test_runner/test_runner_for_specific_view.cc
@@ -559,7 +559,8 @@ void TestRunnerForSpecificView::DidLosePointerLockInternal() {
 }
 
 bool TestRunnerForSpecificView::CallShouldCloseOnWebView() {
-  return web_view()->mainFrame()->dispatchBeforeUnloadEvent();
+  return web_view()->mainFrame()->toWebLocalFrame()->dispatchBeforeUnloadEvent(

[nasko, 2016/05/09 21:56:04]

I don't think you can make this assumption here. It is better off being an if
statement to check if the frame is local or remote. You can ping lukasza@, who
has been working on making the test runner work properly in OOPIF mode.

[Łukasz Anforowicz, 2016/05/09 22:43:23]

nasko@ - you are correct that web_view()->mainFrame() might be a WebRemoteFrame
here.  At the same time:

1) It seems that testRunner.callShouldCloseOnWebView() is called only from a
single layout test (fast/events/onbeforeunload-focused-iframe.html) and this
particular test only involves single-origin (file:) frames.

2) There is already one existing binding (i.e. testRunner.findString aka
TestRunnerForSpecificView::FindString) that assumes that main frame is a
WebLocalFrame.

3) I don't see an easy way to allow calling these bindings from any frame
(including frames in another renderer from the main frame) - allowing this would
probably require some IPCs to forward the call to the right renderer (and this
seems like an overkill for future, hypothetical test needs)

Given above, I think the changes in the CL-under-review are almost okay - I
would just add an explicit check if web_view()->mainFrame()->isWebLocalFrame()
and do something smart if it isn't (I guess violently failing a test is best to
avoid surprising future maintainers - a CHECK(false) + a comment might
accomplish this).

[clamy, 2016/05/10 00:39:56]

With the current architecture, you would get a NOTREACHED() if we tried to call
dispatchBeforeUnloadEvent on a WebRemoteFrame. We could either have that or a
CHECK(false) << "This function should only be called when the main frame is a
WebLocalFrame.".

[clamy, 2016/05/10 01:58:08]

Went with CHECK(false).

+      false);
 }
 
 void TestRunnerForSpecificView::SetDomainRelaxationForbiddenForURLScheme(