PlzNavigate: Eliminate test_runner bailing out of layout tests too early

components/test_runner/test_runner.cc

Index: components/test_runner/test_runner.cc
diff --git a/components/test_runner/test_runner.cc b/components/test_runner/test_runner.cc
index 3c6fa3cce493d873ec05126e24eadf8d1c523db5..c24d74264186ec2c62184bb812033567c33966da 100644
--- a/components/test_runner/test_runner.cc
+++ b/components/test_runner/test_runner.cc
@@ -1555,6 +1555,7 @@ TestRunner::TestRunner(TestInterfaces* interfaces)
       main_view_(nullptr),
       mock_content_settings_client_(
           new MockContentSettingsClient(&layout_test_runtime_flags_)),
+      is_navigating_(false),
       credential_manager_client_(new MockCredentialManagerClient),
       mock_screen_orientation_client_(new MockScreenOrientationClient),
       spellcheck_(new SpellCheckClient(this)),
@@ -1586,6 +1587,7 @@ void TestRunner::SetMainView(WebView* web_view) {
 
 void TestRunner::Reset() {
   is_web_platform_tests_mode_ = false;
+  is_navigating_ = false;
   top_loading_frame_ = nullptr;
   layout_test_runtime_flags_.Reset();
   mock_screen_orientation_client_->ResetData();
@@ -1837,6 +1839,7 @@ bool TestRunner::IsFramePartOfMainTestWindow(blink::WebFrame* frame) const {
 }
 
 bool TestRunner::tryToSetTopLoadingFrame(WebFrame* frame) {
+  is_navigating_ = false;

[clamy, 2016/07/19 13:50:34]

Can it be possible for the new frame to be navigating at this point?

[blundell, 2016/07/21 10:02:18]

Yes. The flow is this:

- FrameLoader::startLoad() is invoked. This results in a browser-side navigation
and causes WebFrameClient::didStartLoading() to be invoked, so TestRunner's
|is_navigating_| is true.
- The browser-side navigation sends back an IPC that causes
RenderFrameImpl::OnCommitNavigation() to be invoked.
- That method calls into WebLocalFrame::load(), which causes
FrameLoader::startLoad() to be invoked again. This time it goes through to the
end instead of short-circuiting out early and causes
WebFrameClient::didStartProvisionalLoad() to be invoked, which in turn invokes
TestRunner::tryToSetTopLoadingFrame().

   if (!IsFramePartOfMainTestWindow(frame))
     return false;
 
@@ -1850,6 +1853,7 @@ bool TestRunner::tryToSetTopLoadingFrame(WebFrame* frame) {
 }
 
 bool TestRunner::tryToClearTopLoadingFrame(WebFrame* frame) {
+  is_navigating_ = false;
   if (!IsFramePartOfMainTestWindow(frame))
     return false;
 
@@ -2735,8 +2739,9 @@ void TestRunner::CheckResponseMimeType() {
 
 void TestRunner::NotifyDone() {
   if (layout_test_runtime_flags_.wait_until_done() && !topLoadingFrame() &&
-      work_queue_.is_empty())
+      !is_navigating_ && work_queue_.is_empty()) {
     delegate_->TestFinished();
+  }
   layout_test_runtime_flags_.set_wait_until_done(false);
   OnLayoutTestRuntimeFlagsChanged();
 }