[chromedriver] Track pending frame id after Page.navigation responses.

This makes ChromeDriver's Navigate command (driver.get) wait for the page to
load, rather than return prematurely. It also fixes
ChromeDriverTest.testEmulateNetworkConditionsOffline and
testSwitchesToTopFrameAfterNavigation, which have been failing on our waterfall
recently (due to the premature return).

BUG=chromedriver:1158

Review URL: https://codereview.chromium.org/1266453002

Cr-Commit-Position: refs/heads/master@{#340801}

8 files changed, 47 insertions, 26 deletions

diff --git a/chrome/test/chromedriver/chrome/devtools_client_impl.cc b/chrome/test/chromedriver/chrome/devtools_client_impl.cc
index cd5c7f8..e33b54c 100644
--- a/chrome/test/chromedriver/chrome/devtools_client_impl.cc
+++ b/chrome/test/chromedriver/chrome/devtools_client_impl.cc
@@ -447,8 +447,10 @@ Status DevToolsClientImpl::EnsureListenersNotifiedOfCommandResponse() {
     DevToolsEventListener* listener =
         unnotified_cmd_response_listeners_.front();
     unnotified_cmd_response_listeners_.pop_front();
-    Status status =
-        listener->OnCommandSuccess(this, unnotified_cmd_response_info_->method);
+    Status status = listener->OnCommandSuccess(
+        this,
+        unnotified_cmd_response_info_->method,
+        *unnotified_cmd_response_info_->response.result.get());
     if (status.IsError())
       return status;
   }
diff --git a/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc b/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc
index 4c9656d..fb70d8d 100644
--- a/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc
+++ b/chrome/test/chromedriver/chrome/devtools_client_impl_unittest.cc
@@ -1066,7 +1066,8 @@ class MockCommandListener : public DevToolsEventListener {
   }
 
   Status OnCommandSuccess(DevToolsClient* client,
-                          const std::string& method) override {
+                          const std::string& method,
+                          const base::DictionaryValue& result) override {
     msgs_.push_back(method);
     if (!callback_.is_null())
       callback_.Run(client);
diff --git a/chrome/test/chromedriver/chrome/devtools_event_listener.cc b/chrome/test/chromedriver/chrome/devtools_event_listener.cc
index d21149e..3077c31 100644
--- a/chrome/test/chromedriver/chrome/devtools_event_listener.cc
+++ b/chrome/test/chromedriver/chrome/devtools_event_listener.cc
@@ -20,7 +20,8 @@ Status DevToolsEventListener::OnEvent(DevToolsClient* client,
 
 Status DevToolsEventListener::OnCommandSuccess(
     DevToolsClient* client,
-    const std::string& method) {
+    const std::string& method,
+    const base::DictionaryValue& result) {
   return Status(kOk);
 }
 
diff --git a/chrome/test/chromedriver/chrome/devtools_event_listener.h b/chrome/test/chromedriver/chrome/devtools_event_listener.h
index 2d2f159..51c09a8 100644
--- a/chrome/test/chromedriver/chrome/devtools_event_listener.h
+++ b/chrome/test/chromedriver/chrome/devtools_event_listener.h
@@ -30,7 +30,8 @@ class DevToolsEventListener {
 
   // Called when a command success response is received.
   virtual Status OnCommandSuccess(DevToolsClient* client,
-                                  const std::string& method);
+                                  const std::string& method,
+                                  const base::DictionaryValue& result);
 
   // True if the listener should be added to the browser-wide |DevToolsClient|
   // in addition to all webview |DevToolsClient|s. False by default. If set to
diff --git a/chrome/test/chromedriver/chrome/navigation_tracker.cc b/chrome/test/chromedriver/chrome/navigation_tracker.cc
index 9e7fadd..2d37fb5 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker.cc
+++ b/chrome/test/chromedriver/chrome/navigation_tracker.cc
@@ -53,7 +53,8 @@ Status NavigationTracker::IsPendingNavigation(const std::string& frame_id,
     std::string ready_state;
     if (status.IsError() || !result->GetString("result.value", &ready_state))
       return status;
-    *is_pending = ready_state != "complete";
+    if (ready_state == "complete")
+      loading_state_ = kNotLoading;
   } else {
     if (loading_state_ == kUnknown) {
       scoped_ptr<base::DictionaryValue> result;
@@ -104,8 +105,8 @@ Status NavigationTracker::IsPendingNavigation(const std::string& frame_id,
       if (loading_state_ == kUnknown)
         loading_state_ = kLoading;
     }
-    *is_pending = loading_state_ == kLoading;
   }
+  *is_pending = loading_state_ == kLoading;
 
   if (frame_id.empty()) {
     *is_pending |= scheduled_frame_set_.size() > 0;
@@ -198,14 +199,33 @@ Status NavigationTracker::OnEvent(DevToolsClient* client,
       pending_frame_set_.clear();
       scheduled_frame_set_.clear();
     }
-  } else if (method == "Inspector.targetCrashed") {
+  } else if (method == "Runtime.executionContextsCleared" ||
+             method == "Runtime.executionContextDestroyed") {
+    ResetLoadingState(kLoading);
+  } else if (method == "Page.loadEventFired" ||
+             method == "Inspector.targetCrashed") {
     ResetLoadingState(kNotLoading);
   }
   return Status(kOk);
 }
 
-Status NavigationTracker::OnCommandSuccess(DevToolsClient* client,
-                                           const std::string& method) {
+Status NavigationTracker::OnCommandSuccess(
+    DevToolsClient* client,
+    const std::string& method,
+    const base::DictionaryValue& result) {
+  if (method == "Page.navigate" && loading_state_ == kLoading) {
+    // In all versions of Chrome that are supported by ChromeDriver, except for
+    // old versions of Android WebView, Page.navigate will return a frameId in
+    // the command response. We'll get a notification that the frame has loaded
+    // when we get the Page.frameStoppedLoading event, so keep track of the
+    // pending frame id here.
+    std::string pending_frame_id;
+    if (result.GetString("frameId", &pending_frame_id)) {
+      pending_frame_set_.insert(pending_frame_id);
+      return Status(kOk);
+    }
+  }
+
   if ((method == "Page.navigate" || method == "Page.navigateToHistoryEntry") &&
       loading_state_ != kLoading) {
     // At this point the browser has initiated the navigation, but besides that,
diff --git a/chrome/test/chromedriver/chrome/navigation_tracker.h b/chrome/test/chromedriver/chrome/navigation_tracker.h
index a75a094..f7b84f1 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker.h
+++ b/chrome/test/chromedriver/chrome/navigation_tracker.h
@@ -49,7 +49,8 @@ class NavigationTracker : public DevToolsEventListener {
                  const std::string& method,
                  const base::DictionaryValue& params) override;
   Status OnCommandSuccess(DevToolsClient* client,
-                          const std::string& method) override;
+                          const std::string& method,
+                          const base::DictionaryValue& result) override;
 
  private:
   DevToolsClient* client_;
diff --git a/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc b/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc
index 2fd2fc8..6c092ad 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc
+++ b/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc
@@ -333,24 +333,15 @@ TEST(NavigationTracker, UnknownStateForcesStartReceivesStop) {
 
 TEST(NavigationTracker, OnSuccessfulNavigate) {
   base::DictionaryValue params;
-  params.SetString("frameId", "f");
-  DeterminingLoadStateDevToolsClient client(
-      false, true, "Page.frameStoppedLoading", &params);
-  BrowserInfo browser_info;
-  NavigationTracker tracker(
-      &client, NavigationTracker::kNotLoading, &browser_info);
-  tracker.OnCommandSuccess(&client, "Page.navigate");
-  ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", false));
-}
-
-TEST(NavigationTracker, OnSuccessfulNavigateStillWaiting) {
-  base::DictionaryValue params;
-  params.SetString("frameId", "f");
   DeterminingLoadStateDevToolsClient client(
       false, true, std::string(), &params);
   BrowserInfo browser_info;
   NavigationTracker tracker(
       &client, NavigationTracker::kNotLoading, &browser_info);
-  tracker.OnCommandSuccess(&client, "Page.navigate");
+  base::DictionaryValue result;
+  result.SetString("frameId", "f");
+  tracker.OnCommandSuccess(&client, "Page.navigate", result);
   ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", true));
+  tracker.OnEvent(&client, "Page.loadEventFired", params);
+  ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", false));
 }
diff --git a/chrome/test/chromedriver/test/run_py_tests.py b/chrome/test/chromedriver/test/run_py_tests.py
index 49ac70e..c0e301b 100755
--- a/chrome/test/chromedriver/test/run_py_tests.py
+++ b/chrome/test/chromedriver/test/run_py_tests.py
@@ -54,7 +54,7 @@ _NEGATIVE_FILTER = [
     'ChromeDriverTest.testEmulateNetworkConditionsNameSpeed',
     'ChromeDriverTest.testEmulateNetworkConditionsSpeed',
     # crbug.com/469947
-    'ChromeDriverTest.testTouchPinch'
+    'ChromeDriverTest.testTouchPinch',
     # https://code.google.com/p/chromedriver/issues/detail?id=1167
     'ChromeDriverTest.testShouldHandleNewWindowLoadingProperly',
 ]
@@ -1105,6 +1105,10 @@ class ChromeDriverTest(ChromeDriverBaseTest):
     a.Click()
     self.assertTrue(self._driver.GetCurrentUrl().endswith('#two'))
 
+  def testDoesntHangOnFragmentNavigation(self):
+    self._driver.Load(self.GetHttpUrlForFile('/chromedriver/empty.html'))
+    self._driver.Load(self.GetHttpUrlForFile('/chromedriver/empty.html#x'))
+
 
 class ChromeDriverAndroidTest(ChromeDriverBaseTest):
   """End to end tests for Android-specific tests."""