[chromedriver] Navigation tracker improvements.

Track when execution contexts are created and destroyed. This prevents the
navigation tracker from hanging when iframes are added dynamically to a page.

Avoid checking if document.readyState == "complete", since this can fail if
pages forget to call document.close() after calling document.write().

Always check for a dummy page by calling DOM.getDocument and checking baseURL,
including on Chrome 44+.

If a window command fails while a new page or frame started loading, retry
the command after the pending navigation has completed.

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

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

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

4 files changed, 147 insertions, 138 deletions

diff --git a/chrome/test/chromedriver/chrome/navigation_tracker.cc b/chrome/test/chromedriver/chrome/navigation_tracker.cc
index ae77138..46f5b16 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker.cc
+++ b/chrome/test/chromedriver/chrome/navigation_tracker.cc
@@ -31,68 +31,57 @@ NavigationTracker::~NavigationTracker() {}
 
 Status NavigationTracker::IsPendingNavigation(const std::string& frame_id,
                                               bool* is_pending) {
-  if (!IsExpectingFrameLoadingEvents()) {
-    if (loading_state_ != kLoading) {
-      base::DictionaryValue params;
-      params.SetString("expression", "document.readyState");
-      scoped_ptr<base::DictionaryValue> result;
-      Status status =
-          client_->SendCommandAndGetResult("Runtime.evaluate", params, &result);
-      std::string ready_state;
-      if (status.IsError() || !result->GetString("result.value", &ready_state))
-        return status;
-      loading_state_ = ready_state == "complete" ? kNotLoading : kLoading;
+  if (loading_state_ == kUnknown ||
+      (loading_state_ == kNotLoading && !IsExpectingFrameLoadingEvents())) {
+    // In the case that a http request is sent to server to fetch the page
+    // content and the server hasn't responded at all, a dummy page is created
+    // for the new window. In such case, the baseURL will be empty.
+    base::DictionaryValue empty_params;
+    scoped_ptr<base::DictionaryValue> result;
+    Status status = client_->SendCommandAndGetResult(
+        "DOM.getDocument", empty_params, &result);
+    std::string base_url;
+    if (status.IsError() || !result->GetString("root.baseURL", &base_url))
+      return Status(kUnknownError, "cannot determine loading status", status);
+    if (base_url.empty()) {
+      *is_pending = true;
+      loading_state_ = kLoading;
+      return Status(kOk);
     }
-  } else {
-    if (loading_state_ == kUnknown) {
-      scoped_ptr<base::DictionaryValue> result;
-
-      // In the case that a http request is sent to server to fetch the page
-      // content and the server hasn't responded at all, a dummy page is created
-      // for the new window. In such case, the baseURL will be empty.
-      base::DictionaryValue empty_params;
-      Status status = client_->SendCommandAndGetResult(
-          "DOM.getDocument", empty_params, &result);
-      std::string base_url;
-      if (status.IsError() || !result->GetString("root.baseURL", &base_url))
-        return Status(kUnknownError, "cannot determine loading status", status);
-      if (base_url.empty()) {
-        *is_pending = true;
-        loading_state_ = kLoading;
-        return Status(kOk);
-      }
+  }
 
-      // If the loading state is unknown (which happens after first connecting),
-      // force loading to start and set the state to loading. This will cause a
-      // frame start event to be received, and the frame stop event will not be
-      // received until all frames are loaded.  Loading is forced to start by
-      // attaching a temporary iframe.  Forcing loading to start is not
-      // necessary if the main frame is not yet loaded.
-      const char kStartLoadingIfMainFrameNotLoading[] =
-         "var isLoaded = document.readyState == 'complete' ||"
-         "    document.readyState == 'interactive';"
-         "if (isLoaded) {"
-         "  var frame = document.createElement('iframe');"
-         "  frame.src = 'about:blank';"
-         "  document.body.appendChild(frame);"
-         "  window.setTimeout(function() {"
-         "    document.body.removeChild(frame);"
-         "  }, 0);"
-         "}";
-      base::DictionaryValue params;
-      params.SetString("expression", kStartLoadingIfMainFrameNotLoading);
-      status = client_->SendCommandAndGetResult(
-          "Runtime.evaluate", params, &result);
-      if (status.IsError())
-        return Status(kUnknownError, "cannot determine loading status", status);
+  if (loading_state_ == kUnknown) {
+    // If the loading state is unknown (which happens after first connecting),
+    // force loading to start and set the state to loading. This will cause a
+    // frame start event to be received, and the frame stop event will not be
+    // received until all frames are loaded.  Loading is forced to start by
+    // attaching a temporary iframe.  Forcing loading to start is not
+    // necessary if the main frame is not yet loaded.
+    const char kStartLoadingIfMainFrameNotLoading[] =
+       "var isLoaded = document.readyState == 'complete' ||"
+       "    document.readyState == 'interactive';"
+       "if (isLoaded) {"
+       "  var frame = document.createElement('iframe');"
+       "  frame.src = 'about:blank';"
+       "  document.body.appendChild(frame);"
+       "  window.setTimeout(function() {"
+       "    document.body.removeChild(frame);"
+       "  }, 0);"
+       "}";
+    base::DictionaryValue params;
+    params.SetString("expression", kStartLoadingIfMainFrameNotLoading);
+    scoped_ptr<base::DictionaryValue> result;
+    Status status = client_->SendCommandAndGetResult(
+        "Runtime.evaluate", params, &result);
+    if (status.IsError())
+      return Status(kUnknownError, "cannot determine loading status", status);
 
-      // Between the time the JavaScript is evaluated and
-      // SendCommandAndGetResult returns, OnEvent may have received info about
-      // the loading state.  This is only possible during a nested command. Only
-      // set the loading state if the loading state is still unknown.
-      if (loading_state_ == kUnknown)
-        loading_state_ = kLoading;
-    }
+    // Between the time the JavaScript is evaluated and
+    // SendCommandAndGetResult returns, OnEvent may have received info about
+    // the loading state.  This is only possible during a nested command. Only
+    // set the loading state if the loading state is still unknown.
+    if (loading_state_ == kUnknown)
+      loading_state_ = kLoading;
   }
   *is_pending = loading_state_ == kLoading;
 
@@ -187,10 +176,27 @@ Status NavigationTracker::OnEvent(DevToolsClient* client,
       pending_frame_set_.clear();
       scheduled_frame_set_.clear();
     }
-  } else if (method == "Runtime.executionContextsCleared" ||
-             method == "Runtime.executionContextDestroyed") {
-    if (!IsExpectingFrameLoadingEvents())
+  } else if (method == "Runtime.executionContextsCleared") {
+    if (!IsExpectingFrameLoadingEvents()) {
+      execution_context_set_.clear();
       ResetLoadingState(kLoading);
+    }
+  } else if (method == "Runtime.executionContextCreated") {
+    if (!IsExpectingFrameLoadingEvents()) {
+      int execution_context_id;
+      if (!params.GetInteger("context.id", &execution_context_id))
+        return Status(kUnknownError, "missing or invalid 'context.id'");
+      execution_context_set_.insert(execution_context_id);
+    }
+  } else if (method == "Runtime.executionContextDestroyed") {
+    if (!IsExpectingFrameLoadingEvents()) {
+      int execution_context_id;
+      if (!params.GetInteger("executionContextId", &execution_context_id))
+        return Status(kUnknownError, "missing or invalid 'context.id'");
+      execution_context_set_.erase(execution_context_id);
+      if (execution_context_set_.empty())
+        loading_state_ = kLoading;
+    }
   } else if (method == "Page.loadEventFired") {
     if (!IsExpectingFrameLoadingEvents())
       ResetLoadingState(kNotLoading);
diff --git a/chrome/test/chromedriver/chrome/navigation_tracker.h b/chrome/test/chromedriver/chrome/navigation_tracker.h
index a6fb3c7..6ff66ee 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker.h
+++ b/chrome/test/chromedriver/chrome/navigation_tracker.h
@@ -58,6 +58,7 @@ class NavigationTracker : public DevToolsEventListener {
   const BrowserInfo* browser_info_;
   std::set<std::string> pending_frame_set_;
   std::set<std::string> scheduled_frame_set_;
+  std::set<int> execution_context_set_;
 
   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 596dfcd..71e6421 100644
--- a/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc
+++ b/chrome/test/chromedriver/chrome/navigation_tracker_unittest.cc
@@ -23,10 +23,59 @@ void AssertPendingState(NavigationTracker* tracker,
   ASSERT_EQ(expected_is_pending, is_pending);
 }
 
+class DeterminingLoadStateDevToolsClient : public StubDevToolsClient {
+ public:
+  DeterminingLoadStateDevToolsClient(
+      bool has_empty_base_url,
+      bool is_loading,
+      const std::string& send_event_first,
+      base::DictionaryValue* send_event_first_params)
+      : has_empty_base_url_(has_empty_base_url),
+        is_loading_(is_loading),
+        send_event_first_(send_event_first),
+        send_event_first_params_(send_event_first_params) {}
+
+  ~DeterminingLoadStateDevToolsClient() override {}
+
+  Status SendCommandAndGetResult(
+      const std::string& method,
+      const base::DictionaryValue& params,
+      scoped_ptr<base::DictionaryValue>* result) override {
+    if (method == "DOM.getDocument") {
+      base::DictionaryValue result_dict;
+      if (has_empty_base_url_)
+        result_dict.SetString("root.baseURL", std::string());
+      else
+        result_dict.SetString("root.baseURL", "http://test");
+      result->reset(result_dict.DeepCopy());
+      return Status(kOk);
+    }
+
+    if (send_event_first_.length()) {
+      Status status = listeners_.front()
+          ->OnEvent(this, send_event_first_, *send_event_first_params_);
+      if (status.IsError())
+        return status;
+    }
+
+    base::DictionaryValue result_dict;
+    result_dict.SetBoolean("result.value", is_loading_);
+    result->reset(result_dict.DeepCopy());
+    return Status(kOk);
+  }
+
+ private:
+  bool has_empty_base_url_;
+  bool is_loading_;
+  std::string send_event_first_;
+  base::DictionaryValue* send_event_first_params_;
+};
+
 }  // namespace
 
 TEST(NavigationTracker, FrameLoadStartStop) {
-  StubDevToolsClient client;
+  base::DictionaryValue dict;
+  DeterminingLoadStateDevToolsClient client(false, true, std::string(), &dict);
   BrowserInfo browser_info;
   NavigationTracker tracker(&client, &browser_info);
 
@@ -45,7 +94,8 @@ TEST(NavigationTracker, FrameLoadStartStop) {
 // can sometimes see two Page.frameStartedLoading events with only a single
 // Page.frameStoppedLoading event.
 TEST(NavigationTracker, FrameLoadStartStartStop) {
-  StubDevToolsClient client;
+  base::DictionaryValue dict;
+  DeterminingLoadStateDevToolsClient client(false, true, std::string(), &dict);
   BrowserInfo browser_info;
   NavigationTracker tracker(&client, &browser_info);
 
@@ -64,7 +114,8 @@ TEST(NavigationTracker, FrameLoadStartStartStop) {
 }
 
 TEST(NavigationTracker, MultipleFramesLoad) {
-  StubDevToolsClient client;
+  base::DictionaryValue dict;
+  DeterminingLoadStateDevToolsClient client(false, true, std::string(), &dict);
   BrowserInfo browser_info;
   NavigationTracker tracker(&client, &browser_info);
   base::DictionaryValue params;
@@ -102,7 +153,8 @@ TEST(NavigationTracker, MultipleFramesLoad) {
 }
 
 TEST(NavigationTracker, NavigationScheduledThenLoaded) {
-  StubDevToolsClient client;
+  base::DictionaryValue dict;
+  DeterminingLoadStateDevToolsClient client(false, true, std::string(), &dict);
   BrowserInfo browser_info;
   NavigationTracker tracker(
       &client, NavigationTracker::kNotLoading, &browser_info);
@@ -131,7 +183,8 @@ TEST(NavigationTracker, NavigationScheduledThenLoaded) {
 }
 
 TEST(NavigationTracker, NavigationScheduledForOtherFrame) {
-  StubDevToolsClient client;
+  base::DictionaryValue dict;
+  DeterminingLoadStateDevToolsClient client(false, true, std::string(), &dict);
   BrowserInfo browser_info;
   NavigationTracker tracker(
       &client, NavigationTracker::kNotLoading, &browser_info);
@@ -147,7 +200,8 @@ TEST(NavigationTracker, NavigationScheduledForOtherFrame) {
 }
 
 TEST(NavigationTracker, NavigationScheduledThenCancelled) {
-  StubDevToolsClient client;
+  base::DictionaryValue dict;
+  DeterminingLoadStateDevToolsClient client(false, true, std::string(), &dict);
   BrowserInfo browser_info;
   NavigationTracker tracker(
       &client, NavigationTracker::kNotLoading, &browser_info);
@@ -170,7 +224,8 @@ TEST(NavigationTracker, NavigationScheduledThenCancelled) {
 }
 
 TEST(NavigationTracker, NavigationScheduledTooFarAway) {
-  StubDevToolsClient client;
+  base::DictionaryValue dict;
+  DeterminingLoadStateDevToolsClient client(false, true, std::string(), &dict);
   BrowserInfo browser_info;
   NavigationTracker tracker(
       &client, NavigationTracker::kNotLoading, &browser_info);
@@ -186,7 +241,8 @@ TEST(NavigationTracker, NavigationScheduledTooFarAway) {
 }
 
 TEST(NavigationTracker, DiscardScheduledNavigationsOnMainFrameCommit) {
-  StubDevToolsClient client;
+  base::DictionaryValue dict;
+  DeterminingLoadStateDevToolsClient client(false, true, std::string(), &dict);
   BrowserInfo browser_info;
   NavigationTracker tracker(
       &client, NavigationTracker::kNotLoading, &browser_info);
@@ -251,72 +307,6 @@ TEST(NavigationTracker, UnknownStateFailsToDetermineState) {
             tracker.IsPendingNavigation("f", &is_pending).code());
 }
 
-namespace {
-
-class DeterminingLoadStateDevToolsClient : public StubDevToolsClient {
- public:
-  DeterminingLoadStateDevToolsClient(
-      bool has_empty_base_url,
-      bool is_loading,
-      const std::string& send_event_first,
-      base::DictionaryValue* send_event_first_params)
-      : has_empty_base_url_(has_empty_base_url),
-        is_loading_(is_loading),
-        send_event_first_(send_event_first),
-        send_event_first_params_(send_event_first_params) {}
-
-  ~DeterminingLoadStateDevToolsClient() override {}
-
-  Status SendCommandAndGetResult(
-      const std::string& method,
-      const base::DictionaryValue& params,
-      scoped_ptr<base::DictionaryValue>* result) override {
-    if (method == "DOM.getDocument") {
-      base::DictionaryValue result_dict;
-      if (has_empty_base_url_)
-        result_dict.SetString("root.baseURL", std::string());
-      else
-        result_dict.SetString("root.baseURL", "http://test");
-      result->reset(result_dict.DeepCopy());
-      return Status(kOk);
-    } else if (method == "Runtime.evaluate") {
-      std::string expression;
-      params.GetString("expression", &expression);
-      if (expression == "document.readyState") {
-        base::DictionaryValue result_dict;
-        if (is_loading_) {
-          result_dict.SetString("result.value", "loading");
-          is_loading_ = false;
-        } else {
-          result_dict.SetString("result.value", "complete");
-        }
-        result->reset(result_dict.DeepCopy());
-        return Status(kOk);
-      }
-    }
-
-    if (send_event_first_.length()) {
-      Status status = listeners_.front()
-          ->OnEvent(this, send_event_first_, *send_event_first_params_);
-      if (status.IsError())
-        return status;
-    }
-
-    base::DictionaryValue result_dict;
-    result_dict.SetBoolean("result.value", is_loading_);
-    result->reset(result_dict.DeepCopy());
-    return Status(kOk);
-  }
-
- private:
-  bool has_empty_base_url_;
-  bool is_loading_;
-  std::string send_event_first_;
-  base::DictionaryValue* send_event_first_params_;
-};
-
-}  // namespace
-
 TEST(NavigationTracker, UnknownStatePageNotLoadAtAll) {
   base::DictionaryValue params;
   DeterminingLoadStateDevToolsClient client(
diff --git a/chrome/test/chromedriver/window_commands.cc b/chrome/test/chromedriver/window_commands.cc
index 259da82..9ef8d0c 100644
--- a/chrome/test/chromedriver/window_commands.cc
+++ b/chrome/test/chromedriver/window_commands.cc
@@ -223,8 +223,20 @@ Status ExecuteWindowCommand(
       return nav_status;
 
     status = command.Run(session, web_view, params, value);
-    if (status.code() != kNoSuchExecutionContext)
-      break;
+    if (status.code() == kNoSuchExecutionContext) {
+      continue;
+    } else if (status.IsError()) {
+      // If the command failed while a new page or frame started loading, retry
+      // the command after the pending navigation has completed.
+      bool is_pending = false;
+      nav_status = web_view->IsPendingNavigation(session->GetCurrentFrameId(),
+                                                 &is_pending);
+      if (nav_status.IsError())
+        return nav_status;
+      else if (is_pending)
+        continue;
+    }
+    break;
   }
 
   nav_status = web_view->WaitForPendingNavigations(