[chromedriver] Track if the load event has fired for the current execution context.

If we haven't seen a load event yet, but we're still expecting one, keep
waiting. This prevents ChromeDriver commands from returning prematurely.

We also need to be careful to avoid waiting for the load event if we've called
window.stop() due to a timeout. Calling window.stop() will prevent the load
event from firing, so if the page has timed out, don't wait for the load event.

BUG=chromedriver:1167
TBR=stgao@chromium.org

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

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

4 files changed, 33 insertions, 12 deletions

diff --git a/chrome/test/chromedriver/chrome/navigation_tracker.cc b/chrome/test/chromedriver/chrome/navigation_tracker.cc
index 73486e3..a557b6b 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker.cc
+++ b/chrome/test/chromedriver/chrome/navigation_tracker.cc
@@ -22,7 +22,9 @@ NavigationTracker::NavigationTracker(DevToolsClient* client,
     : client_(client),
       loading_state_(kUnknown),
       browser_info_(browser_info),
-      dummy_execution_context_id_(0) {
+      dummy_execution_context_id_(0),
+      load_event_fired_(true),
+      timed_out_(false) {
   client_->AddListener(this);
 }
 
@@ -32,7 +34,9 @@ NavigationTracker::NavigationTracker(DevToolsClient* client,
     : client_(client),
       loading_state_(known_state),
       browser_info_(browser_info),
-      dummy_execution_context_id_(0) {
+      dummy_execution_context_id_(0),
+      load_event_fired_(true),
+      timed_out_(false) {
   client_->AddListener(this);
 }
 
@@ -100,6 +104,10 @@ Status NavigationTracker::IsPendingNavigation(const std::string& frame_id,
   return Status(kOk);
 }
 
+void NavigationTracker::set_timed_out(bool timed_out) {
+  timed_out_ = timed_out;
+}
+
 Status NavigationTracker::OnConnected(DevToolsClient* client) {
   ResetLoadingState(kUnknown);
 
@@ -144,11 +152,10 @@ Status NavigationTracker::OnEvent(DevToolsClient* client,
       return Status(kUnknownError, "missing or invalid 'frameId'");
 
     pending_frame_set_.erase(frame_id);
-
-    if (pending_frame_set_.empty() || expecting_single_stop_event) {
+    if (expecting_single_stop_event)
       pending_frame_set_.clear();
+    if (pending_frame_set_.empty() && (load_event_fired_ || timed_out_))
       loading_state_ = kNotLoading;
-    }
   } else if (method == "Page.frameScheduledNavigation") {
     double delay;
     if (!params.GetDouble("delay", &delay))
@@ -174,12 +181,14 @@ Status NavigationTracker::OnEvent(DevToolsClient* client,
 
     const base::Value* unused_value;
     if (!params.Get("frame.parentId", &unused_value)) {
-      // If the main frame just navigated, discard any pending scheduled
-      // navigations. For some reasons at times the cleared event is not
-      // received when navigating.
-      // See crbug.com/180742.
-      pending_frame_set_.clear();
-      scheduled_frame_set_.clear();
+      if (IsExpectingFrameLoadingEvents()) {
+        // If the main frame just navigated, discard any pending scheduled
+        // navigations. For some reasons at times the cleared event is not
+        // received when navigating.
+        // See crbug.com/180742.
+        pending_frame_set_.clear();
+        scheduled_frame_set_.clear();
+      }
     } else {
       // If a child frame just navigated, check if it is the dummy frame that
       // was attached by IsPendingNavigation(). We don't want to track execution
@@ -194,6 +203,7 @@ Status NavigationTracker::OnEvent(DevToolsClient* client,
     if (!IsExpectingFrameLoadingEvents()) {
       execution_context_set_.clear();
       ResetLoadingState(kLoading);
+      load_event_fired_ = false;
     }
   } else if (method == "Runtime.executionContextCreated") {
     if (!IsExpectingFrameLoadingEvents()) {
@@ -217,6 +227,7 @@ Status NavigationTracker::OnEvent(DevToolsClient* client,
       if (execution_context_id != dummy_execution_context_id_) {
         if (execution_context_set_.empty()) {
           loading_state_ = kLoading;
+          load_event_fired_ = false;
           dummy_frame_id_ = std::string();
           dummy_execution_context_id_ = 0;
         }
@@ -224,7 +235,7 @@ Status NavigationTracker::OnEvent(DevToolsClient* client,
     }
   } else if (method == "Page.loadEventFired") {
     if (!IsExpectingFrameLoadingEvents())
-      ResetLoadingState(kNotLoading);
+      load_event_fired_ = true;
   } else if (method == "Inspector.targetCrashed") {
     ResetLoadingState(kNotLoading);
   }
diff --git a/chrome/test/chromedriver/chrome/navigation_tracker.h b/chrome/test/chromedriver/chrome/navigation_tracker.h
index ac7b695..54892baf 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker.h
+++ b/chrome/test/chromedriver/chrome/navigation_tracker.h
@@ -43,6 +43,8 @@ class NavigationTracker : public DevToolsEventListener {
   // may be empty to signify the main frame.
   Status IsPendingNavigation(const std::string& frame_id, bool* is_pending);
 
+  void set_timed_out(bool timed_out);
+
   // Overridden from DevToolsEventListener:
   Status OnConnected(DevToolsClient* client) override;
   Status OnEvent(DevToolsClient* client,
@@ -61,6 +63,8 @@ class NavigationTracker : public DevToolsEventListener {
   std::set<int> execution_context_set_;
   std::string dummy_frame_id_;
   int dummy_execution_context_id_;
+  bool load_event_fired_;
+  bool timed_out_;
 
   void ResetLoadingState(LoadingState loading_state);
   bool IsExpectingFrameLoadingEvents();
diff --git a/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc b/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc
index 0c954e9..f47d5d3 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc
+++ b/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc
@@ -351,5 +351,9 @@ TEST(NavigationTracker, OnSuccessfulNavigate) {
   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", true));
+  params.Clear();
+  params.SetString("frameId", "f");
+  tracker.OnEvent(&client, "Page.frameStoppedLoading", params);
   ASSERT_NO_FATAL_FAILURE(AssertPendingState(&tracker, "f", false));
 }
diff --git a/chrome/test/chromedriver/chrome/web_view_impl.cc b/chrome/test/chromedriver/chrome/web_view_impl.cc
index c87f139..0c37a57 100644
--- a/chrome/test/chromedriver/chrome/web_view_impl.cc
+++ b/chrome/test/chromedriver/chrome/web_view_impl.cc
@@ -407,11 +407,13 @@ Status WebViewImpl::WaitForPendingNavigations(const std::string& frame_id,
   if (status.code() == kTimeout && stop_load_on_timeout) {
     VLOG(0) << "Timed out. Stopping navigation...";
     scoped_ptr<base::Value> unused_value;
+    navigation_tracker_->set_timed_out(true);
     EvaluateScript(std::string(), "window.stop();", &unused_value);
     Status new_status = client_->HandleEventsUntil(
         base::Bind(&WebViewImpl::IsNotPendingNavigation, base::Unretained(this),
                    frame_id),
         base::TimeDelta::FromSeconds(10));
+    navigation_tracker_->set_timed_out(false);
     if (new_status.IsError())
       status = new_status;
   }