[ChromeDriver] Wait for Current Frame to load

When Waiting for Pending Navigation, code will respond to loading events
for the current frame as well as the top frame. ChromeDriver will
therefore wait for changes in the current frame to complete before
running the next command.

Bug: chromedriver:3164
Change-Id: Id9000afcb7e7a7689466c395071dcccc09c8b23d
Fixed: chromedriver:3164
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1859637Reviewed-by: John Chen <johnchen@chromium.org>
Commit-Queue: Tricia Crichton <triciac@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716204}

chrome/test/chromedriver/chrome/chrome_desktop_impl.cc

@@ -153,10 +153,10 @@ Status ChromeDesktopImpl::WaitForPageToLoad(
     // https://code.google.com/p/chromedriver/issues/detail?id=1205
     device_metrics = nullptr;
   }
-  std::unique_ptr<WebView> web_view_tmp(
-      new WebViewImpl(id, w3c_compliant, devtools_http_client_->browser_info(),
-                      devtools_http_client_->CreateClient(id), device_metrics,
-                      page_load_strategy()));
+  std::unique_ptr<WebView> web_view_tmp(new WebViewImpl(
+      id, w3c_compliant, nullptr, devtools_http_client_->browser_info(),
+      devtools_http_client_->CreateClient(id), device_metrics,
+      page_load_strategy()));
   Status status = web_view_tmp->ConnectIfNecessary();
   if (status.IsError())
     return status;

chrome/test/chromedriver/chrome/chrome_impl.cc

@@ -100,9 +100,9 @@ void ChromeImpl::UpdateWebViews(const WebViewsInfo& views_info,
         // OnConnected will fire when DevToolsClient connects later.
         CHECK(!page_load_strategy_.empty());
         web_views_.push_back(std::make_unique<WebViewImpl>(
-            view.id, w3c_compliant, devtools_http_client_->browser_info(),
-            std::move(client), devtools_http_client_->device_metrics(),
-            page_load_strategy_));
+            view.id, w3c_compliant, nullptr,
+            devtools_http_client_->browser_info(), std::move(client),
+            devtools_http_client_->device_metrics(), page_load_strategy_));
       }
     }
   }

chrome/test/chromedriver/chrome/devtools_client_impl.cc

@@ -278,9 +278,18 @@ Status DevToolsClientImpl::HandleEventsUntil(
         return Status(kOk);
     }
 
-    Status status = ProcessNextMessage(-1, timeout);
-    if (status.IsError())
+    // Create a small timeout so conditional_func can be retried
+    // when only funcinterval has expired, continue while loop
+    // but return timeout status if primary timeout has expired
+    Timeout funcinterval =
+        Timeout(base::TimeDelta::FromMilliseconds(100), &timeout);
+    Status status = ProcessNextMessage(-1, funcinterval);
+    if (status.code() == kTimeout) {
+      if (timeout.IsExpired())
+        return status;
+    } else if (status.IsError()) {
       return status;
+    }
   }
 }
 

chrome/test/chromedriver/chrome/devtools_http_client.cc

@@ -201,8 +201,8 @@ Status DevToolsHttpClient::CloseFrontends(const std::string& for_client_id) {
     std::unique_ptr<DevToolsClient> client(new DevToolsClientImpl(
         socket_factory_, endpoint_.GetDebuggerUrl(*it), *it));
     std::unique_ptr<WebViewImpl> web_view(
-        new WebViewImpl(*it, false, &browser_info_, std::move(client), NULL,
-                        page_load_strategy_));
+        new WebViewImpl(*it, false, nullptr, &browser_info_, std::move(client),
+                        nullptr, page_load_strategy_));
 
     status = web_view->ConnectIfNecessary();
     // Ignore disconnected error, because the debugger might have closed when

chrome/test/chromedriver/chrome/navigation_tracker.cc

@@ -27,6 +27,8 @@ Status MakeNavigationCheckFailedStatus(Status command_status) {
     return Status(kUnexpectedAlertOpen);
   else if (command_status.code() == kTimeout)
     return Status(kTimeout);
+  else if (command_status.code() == kNoSuchExecutionContext)
+    return Status(kNoSuchExecutionContext);
   else
     return Status(kUnknownError, "cannot determine loading status",
                   command_status);
@@ -36,12 +38,15 @@ Status MakeNavigationCheckFailedStatus(Status command_status) {
 
 NavigationTracker::NavigationTracker(
     DevToolsClient* client,
+    WebView* web_view,
     const BrowserInfo* browser_info,
     const JavaScriptDialogManager* dialog_manager,
     const bool is_eager)
     : client_(client),
       loading_state_(kUnknown),
+      web_view_(web_view),
       top_frame_id_(client->GetId()),
+      current_frame_id_(top_frame_id_),
       dialog_manager_(dialog_manager),
       is_eager_(is_eager),
       timed_out_(false) {
@@ -51,12 +56,15 @@ NavigationTracker::NavigationTracker(
 NavigationTracker::NavigationTracker(
     DevToolsClient* client,
     LoadingState known_state,
+    WebView* web_view,
     const BrowserInfo* browser_info,
     const JavaScriptDialogManager* dialog_manager,
     const bool is_eager)
     : client_(client),
       loading_state_(known_state),
+      web_view_(web_view),
       top_frame_id_(client->GetId()),
+      current_frame_id_(top_frame_id_),
       dialog_manager_(dialog_manager),
       is_eager_(is_eager),
       timed_out_(false) {
@@ -65,6 +73,14 @@ NavigationTracker::NavigationTracker(
 
 NavigationTracker::~NavigationTracker() {}
 
+void NavigationTracker::ClearState(const std::string& new_frame_id) {
+  loading_state_ = kUnknown;
+  if (new_frame_id.empty())
+    current_frame_id_ = top_frame_id_;
+  else
+    current_frame_id_ = new_frame_id;
+}
+
 Status NavigationTracker::IsPendingNavigation(const std::string& frame_id,
                                               const Timeout* timeout,
                                               bool* is_pending) {
@@ -76,7 +92,6 @@ Status NavigationTracker::IsPendingNavigation(const std::string& frame_id,
     *is_pending = false;
     return Status(kOk);
   }
-
   // Some DevTools commands (e.g. Input.dispatchMouseEvent) are handled in the
   // browser process, and may cause the renderer process to start a new
   // navigation. We need to call Runtime.evaluate to force a roundtrip to the
@@ -123,7 +138,6 @@ Status NavigationTracker::IsPendingNavigation(const std::string& frame_id,
       loading_state_ = kLoading;
       return Status(kOk);
     }
-
     // If we're loading the ChromeDriver automation extension background page,
     // look for a known function to determine the loading status.
     if (base_url == kAutomationExtensionBackgroundPage) {
@@ -134,7 +148,9 @@ Status NavigationTracker::IsPendingNavigation(const std::string& frame_id,
       loading_state_ = function_exists ? kNotLoading : kLoading;
     }
     status = DetermineUnknownLoadingState();
-    if (status.IsError())
+    if (status.code() == kNoSuchExecutionContext)
+      loading_state_ = kLoading;
+    else if (status.IsError())
       return MakeNavigationCheckFailedStatus(status);
   }
   *is_pending = loading_state_ == kLoading;
@@ -175,25 +191,29 @@ Status NavigationTracker::OnEvent(DevToolsClient* client,
                                   const base::DictionaryValue& params) {
   if (method == "Page.loadEventFired" ||
       (is_eager_ && method == "Page.domContentEventFired")) {
-    loading_state_ = kNotLoading;
+    if (top_frame_id_ == current_frame_id_) {
+      loading_state_ = kNotLoading;
+    } else {
+      return DetermineUnknownLoadingState();
+    }
   } else if (method == "Page.frameStartedLoading") {
-    // If frame that started loading is the top frame
-    // set loading_state_ to loading. If it is a subframe
+    // If frame that started loading is the current frame
+    // set loading_state_ to loading. If it is another subframe
     // the loading state should not change
     std::string frame_id;
     if (!params.GetString("frameId", &frame_id))
       return Status(kUnknownError, "missing or invalid 'frameId'");
-    if (frame_id == top_frame_id_) {
+    if (frame_id == current_frame_id_) {
       loading_state_ = kLoading;
     }
   } else if (method == "Page.frameStoppedLoading") {
     // Sometimes Page.frameStoppedLoading fires without
     // an associated Page.loadEventFired. If this happens
-    // for the top frame, assume loading has finished.
+    // for the current frame, assume loading has finished.
     std::string frame_id;
     if (!params.GetString("frameId", &frame_id))
       return Status(kUnknownError, "missing or invalid 'frameId'");
-    if (frame_id == top_frame_id_) {
+    if (frame_id == current_frame_id_) {
       loading_state_ = kNotLoading;
     }
   } else if (method == "Inspector.targetCrashed") {
@@ -205,15 +225,18 @@ Status NavigationTracker::OnEvent(DevToolsClient* client,
 }
 
 Status NavigationTracker::DetermineUnknownLoadingState() {
-  base::DictionaryValue params;
-  params.SetString("expression", "document.readyState");
-  std::unique_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)) {
+  std::unique_ptr<base::Value> result;
+  Status status = web_view_->EvaluateScript(current_frame_id_,
+                                            "document.readyState", &result);
+  if (status.code() == kNoSuchExecutionContext) {
+    loading_state_ = kLoading;
+    // result is not set in this case, so return here
+    return Status(kOk);
+  } else if (status.IsError()) {
     return MakeNavigationCheckFailedStatus(status);
   }
+
+  std::string ready_state = result->GetString();
   if (ready_state == "complete") {
     loading_state_ = kNotLoading;
   } else {

chrome/test/chromedriver/chrome/navigation_tracker.h

@@ -30,12 +30,14 @@ class NavigationTracker : public DevToolsEventListener,
                           public PageLoadStrategy {
  public:
   NavigationTracker(DevToolsClient* client,
+                    WebView* web_view,
                     const BrowserInfo* browser_info,
                     const JavaScriptDialogManager* dialog_manager,
                     const bool is_eager = false);
 
   NavigationTracker(DevToolsClient* client,
                     LoadingState known_state,
+                    WebView* web_view,
                     const BrowserInfo* browser_info,
                     const JavaScriptDialogManager* dialog_manager,
                     const bool is_eager = false);
@@ -49,6 +51,7 @@ class NavigationTracker : public DevToolsEventListener,
                              const Timeout* timeout,
                              bool* is_pending) override;
   void set_timed_out(bool timed_out) override;
+  void ClearState(const std::string& new_frame_id) override;
   bool IsNonBlocking() const override;
 
   Status CheckFunctionExists(const Timeout* timeout, bool* exists);
@@ -67,7 +70,9 @@ class NavigationTracker : public DevToolsEventListener,
   Status DetermineUnknownLoadingState();
   DevToolsClient* client_;
   LoadingState loading_state_;
+  WebView* web_view_;
   std::string top_frame_id_;
+  std::string current_frame_id_;
   const JavaScriptDialogManager* dialog_manager_;
   const bool is_eager_;
   bool timed_out_;

chrome/test/chromedriver/chrome/non_blocking_navigation_tracker.cc

@@ -16,6 +16,9 @@ Status NonBlockingNavigationTracker::IsPendingNavigation(
 
 void NonBlockingNavigationTracker::set_timed_out(bool timed_out) {}
 
+void NonBlockingNavigationTracker::ClearState(const std::string& new_frame_id) {
+}
+
 bool NonBlockingNavigationTracker::IsNonBlocking() const {
   return true;
 }

chrome/test/chromedriver/chrome/non_blocking_navigation_tracker.h

@@ -11,7 +11,7 @@ class Timeout;
 class Status;
 
 class NonBlockingNavigationTracker : public PageLoadStrategy {
-public:
+ public:
   NonBlockingNavigationTracker() {}
 
   ~NonBlockingNavigationTracker() override;
@@ -21,6 +21,7 @@ public:
                              const Timeout* timeout,
                              bool* is_pending) override;
   void set_timed_out(bool timed_out) override;
+  void ClearState(const std::string& new_frame_id) override;
   bool IsNonBlocking() const override;
 };
 

chrome/test/chromedriver/chrome/page_load_strategy.cc

@@ -15,14 +15,17 @@ const char PageLoadStrategy::kEager[] = "eager";
 PageLoadStrategy* PageLoadStrategy::Create(
     std::string strategy,
     DevToolsClient* client,
+    WebView* web_view,
     const BrowserInfo* browser_info,
     const JavaScriptDialogManager* dialog_manager) {
   if (strategy == kNone) {
     return new NonBlockingNavigationTracker();
   } else if (strategy == kNormal) {
-    return new NavigationTracker(client, browser_info, dialog_manager, false);
+    return new NavigationTracker(client, web_view, browser_info, dialog_manager,
+                                 false);
   } else if (strategy == kEager) {
-    return new NavigationTracker(client, browser_info, dialog_manager, true);
+    return new NavigationTracker(client, web_view, browser_info, dialog_manager,
+                                 true);
   } else {
     NOTREACHED() << "invalid strategy '" << strategy << "'";
     return nullptr;

chrome/test/chromedriver/chrome/page_load_strategy.h

@@ -6,6 +6,7 @@
 #define CHROME_TEST_CHROMEDRIVER_CHROME_PAGE_LOAD_STRATEGY_H_
 
 #include "chrome/test/chromedriver/chrome/status.h"
+#include "chrome/test/chromedriver/chrome/web_view.h"
 
 struct BrowserInfo;
 class DevToolsClient;
@@ -14,8 +15,7 @@ class Status;
 class Timeout;
 
 class PageLoadStrategy {
-
-public:
+ public:
   enum LoadingState {
     kUnknown,
     kLoading,
@@ -27,6 +27,7 @@ public:
   static PageLoadStrategy* Create(
       std::string strategy,
       DevToolsClient* client,
+      WebView* web_view,
       const BrowserInfo* browser_info,
       const JavaScriptDialogManager* dialog_manager);
 
@@ -36,6 +37,8 @@ public:
 
   virtual void set_timed_out(bool timed_out) = 0;
 
+  virtual void ClearState(const std::string& new_frame_id) = 0;
+
   virtual bool IsNonBlocking() const = 0;
 
   // Types of page load strategies.

chrome/test/chromedriver/chrome/stub_web_view.cc

@@ -174,7 +174,7 @@ Status StubWebView::WaitForPendingNavigations(const std::string& frame_id,
 
 Status StubWebView::IsPendingNavigation(const std::string& frame_id,
                                         const Timeout* timeout,
-                                        bool* is_pending) {
+                                        bool* is_pending) const {
   return Status(kOk);
 }
 
@@ -235,7 +235,7 @@ Status StubWebView::SynthesizeScrollGesture(int x,
   return Status(kOk);
 }
 
-bool StubWebView::IsNonBlocking() {
+bool StubWebView::IsNonBlocking() const {
   return false;
 }
 
@@ -254,3 +254,5 @@ std::unique_ptr<base::Value> StubWebView::GetCastSinks() {
 std::unique_ptr<base::Value> StubWebView::GetCastIssueMessage() {
   return std::make_unique<base::Value>();
 }
+
+void StubWebView::ClearNavigationState(const std::string& new_frame_id) {}

chrome/test/chromedriver/chrome/stub_web_view.h

@@ -93,7 +93,7 @@ class StubWebView : public WebView {
                                    bool stop_load_on_timeout) override;
   Status IsPendingNavigation(const std::string& frame_id,
                              const Timeout* timeout,
-                             bool* is_pending) override;
+                             bool* is_pending) const override;
   JavaScriptDialogManager* GetJavaScriptDialogManager() override;
   Status OverrideGeolocation(const Geoposition& geoposition) override;
   Status OverrideNetworkConditions(
@@ -118,11 +118,12 @@ class StubWebView : public WebView {
                                  int y,
                                  int xoffset,
                                  int yoffset) override;
-  bool IsNonBlocking() override;
+  bool IsNonBlocking() const override;
   bool IsOOPIF(const std::string& frame_id) override;
   FrameTracker* GetFrameTracker() const override;
   std::unique_ptr<base::Value> GetCastSinks() override;
   std::unique_ptr<base::Value> GetCastIssueMessage() override;
+  void ClearNavigationState(const std::string& new_frame_id) override;
 
  private:
   std::string id_;

chrome/test/chromedriver/chrome/web_view.h

@@ -194,7 +194,7 @@ class WebView {
   // Returns whether the frame is pending navigation.
   virtual Status IsPendingNavigation(const std::string& frame_id,
                                      const Timeout* timeout,
-                                     bool* is_pending) = 0;
+                                     bool* is_pending) const = 0;
 
   // Returns the JavaScriptDialogManager. Never null.
   virtual JavaScriptDialogManager* GetJavaScriptDialogManager() = 0;
@@ -246,7 +246,7 @@ class WebView {
                                          int xoffset,
                                          int yoffset) = 0;
 
-  virtual bool IsNonBlocking() = 0;
+  virtual bool IsNonBlocking() const = 0;
 
   virtual bool IsOOPIF(const std::string& frame_id) = 0;
 
@@ -255,6 +255,8 @@ class WebView {
   virtual std::unique_ptr<base::Value> GetCastSinks() = 0;
 
   virtual std::unique_ptr<base::Value> GetCastIssueMessage() = 0;
+
+  virtual void ClearNavigationState(const std::string& new_frame_id) = 0;
 };
 
 #endif  // CHROME_TEST_CHROMEDRIVER_CHROME_WEB_VIEW_H_

chrome/test/chromedriver/chrome/web_view_impl.cc

@@ -155,6 +155,7 @@ std::unique_ptr<base::DictionaryValue> GenerateTouchPoint(
 
 WebViewImpl::WebViewImpl(const std::string& id,
                          const bool w3c_compliant,
+                         const WebViewImpl* parent,
                          const BrowserInfo* browser_info,
                          std::unique_ptr<DevToolsClient> client,
                          const DeviceMetrics* device_metrics,
@@ -164,15 +165,11 @@ WebViewImpl::WebViewImpl(const std::string& id,
       browser_info_(browser_info),
       is_locked_(false),
       is_detached_(false),
-      parent_(nullptr),
+      parent_(parent),
       client_(std::move(client)),
       dom_tracker_(new DomTracker(client_.get())),
       frame_tracker_(new FrameTracker(client_.get(), this, browser_info)),
       dialog_manager_(new JavaScriptDialogManager(client_.get(), browser_info)),
-      navigation_tracker_(PageLoadStrategy::Create(page_load_strategy,
-                                                   client_.get(),
-                                                   browser_info,
-                                                   dialog_manager_.get())),
       mobile_emulation_override_manager_(
           new MobileEmulationOverrideManager(client_.get(), device_metrics)),
       geolocation_override_manager_(
@@ -188,6 +185,13 @@ WebViewImpl::WebViewImpl(const std::string& id,
   if (browser_info->is_headless)
     download_directory_override_manager_ =
         std::make_unique<DownloadDirectoryOverrideManager>(client_.get());
+  // Child WebViews should not have their own navigation_tracker, but defer
+  // all related calls to their parent. All WebViews must have either parent_
+  // or navigation_tracker_
+  if (!parent_)
+    navigation_tracker_ = std::unique_ptr<PageLoadStrategy>(
+        PageLoadStrategy::Create(page_load_strategy, client_.get(), this,
+                                 browser_info, dialog_manager_.get()));
   client_->SetOwner(this);
 }
 
@@ -203,12 +207,16 @@ WebViewImpl* WebViewImpl::CreateChild(const std::string& session_id,
       static_cast<DevToolsClientImpl*>(client_.get())->GetRootClient();
   std::unique_ptr<DevToolsClient> child_client(
       std::make_unique<DevToolsClientImpl>(root_client, session_id));
-  WebViewImpl* child = new WebViewImpl(target_id, w3c_compliant_, browser_info_,
-                                       std::move(child_client), nullptr,
-                                       navigation_tracker_->IsNonBlocking()
-                                           ? PageLoadStrategy::kNone
+  WebViewImpl* child = new WebViewImpl(
+      target_id, w3c_compliant_, this, browser_info_, std::move(child_client),
+      nullptr,
+      navigation_tracker_->IsNonBlocking() ? PageLoadStrategy::kNone
                                            : PageLoadStrategy::kNormal);
-  child->parent_ = this;
+  if (navigation_tracker_->IsNonBlocking()) {
+    PageLoadStrategy* pls = navigation_tracker_.get();
+    NavigationTracker* nt = static_cast<NavigationTracker*>(pls);
+    child->client_->AddListener(static_cast<DevToolsEventListener*>(nt));
+  }
   return child;
 }
 
@@ -257,7 +265,7 @@ Status WebViewImpl::Load(const std::string& url, const Timeout* timeout) {
     return Status(kUnknownError, "unsupported protocol");
   base::DictionaryValue params;
   params.SetString("url", url);
-  if (navigation_tracker_->IsNonBlocking()) {
+  if (IsNonBlocking()) {
     // With non-bloakcing navigation tracker, the previous navigation might
     // still be in progress, and this can cause the new navigate command to be
     // ignored on Chrome v63 and above. Stop previous navigation first.
@@ -748,6 +756,10 @@ Status WebViewImpl::AddCookie(const std::string& name,
 Status WebViewImpl::WaitForPendingNavigations(const std::string& frame_id,
                                               const Timeout& timeout,
                                               bool stop_load_on_timeout) {
+  // This function should not be called for child WebViews
+  if (parent_ != nullptr)
+    return Status(kUnsupportedOperation,
+                  "Call WaitForPendingNavigations only on the parent WebView");
   VLOG(0) << "Waiting for pending navigations...";
   const auto not_pending_navigation =
       base::Bind(&WebViewImpl::IsNotPendingNavigation, base::Unretained(this),
@@ -775,9 +787,12 @@ Status WebViewImpl::WaitForPendingNavigations(const std::string& frame_id,
 
 Status WebViewImpl::IsPendingNavigation(const std::string& frame_id,
                                         const Timeout* timeout,
-                                        bool* is_pending) {
-  return
-      navigation_tracker_->IsPendingNavigation(frame_id, timeout, is_pending);
+                                        bool* is_pending) const {
+  if (navigation_tracker_)
+    return navigation_tracker_->IsPendingNavigation(frame_id, timeout,
+                                                    is_pending);
+  else
+    return parent_->IsPendingNavigation(frame_id, timeout, is_pending);
 }
 
 JavaScriptDialogManager* WebViewImpl::GetJavaScriptDialogManager() {
@@ -1105,6 +1120,10 @@ Status WebViewImpl::CallAsyncFunctionInternal(
   }
 }
 
+void WebViewImpl::ClearNavigationState(const std::string& new_frame_id) {
+  navigation_tracker_->ClearState(new_frame_id);
+}
+
 Status WebViewImpl::IsNotPendingNavigation(const std::string& frame_id,
                                            const Timeout* timeout,
                                            bool* is_not_pending) {
@@ -1126,8 +1145,11 @@ Status WebViewImpl::IsNotPendingNavigation(const std::string& frame_id,
   return Status(kOk);
 }
 
-bool WebViewImpl::IsNonBlocking() {
-  return navigation_tracker_->IsNonBlocking();
+bool WebViewImpl::IsNonBlocking() const {
+  if (navigation_tracker_)
+    return navigation_tracker_->IsNonBlocking();
+  else
+    return parent_->IsNonBlocking();
 }
 
 bool WebViewImpl::IsOOPIF(const std::string& frame_id) {

chrome/test/chromedriver/chrome/web_view_impl.h

@@ -40,6 +40,7 @@ class WebViewImpl : public WebView {
  public:
   WebViewImpl(const std::string& id,
               const bool w3c_compliant,
+              const WebViewImpl* parent,
               const BrowserInfo* browser_info,
               std::unique_ptr<DevToolsClient> client,
               const DeviceMetrics* device_metrics,
@@ -133,7 +134,7 @@ class WebViewImpl : public WebView {
                                    bool stop_load_on_timeout) override;
   Status IsPendingNavigation(const std::string& frame_id,
                              const Timeout* timeout,
-                             bool* is_pending) override;
+                             bool* is_pending) const override;
   JavaScriptDialogManager* GetJavaScriptDialogManager() override;
   Status OverrideGeolocation(const Geoposition& geoposition) override;
   Status OverrideNetworkConditions(
@@ -158,11 +159,12 @@ class WebViewImpl : public WebView {
                                  int y,
                                  int xoffset,
                                  int yoffset) override;
-  bool IsNonBlocking() override;
+  bool IsNonBlocking() const override;
   bool IsOOPIF(const std::string& frame_id) override;
   FrameTracker* GetFrameTracker() const override;
   std::unique_ptr<base::Value> GetCastSinks() override;
   std::unique_ptr<base::Value> GetCastIssueMessage() override;
+  void ClearNavigationState(const std::string& new_frame_id) override;
 
   const WebViewImpl* GetParent() const;
   bool Lock();

chrome/test/chromedriver/chrome/web_view_impl_unittest.cc

@@ -8,11 +8,18 @@
 #include <memory>
 #include <string>
 
+#include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "chrome/test/chromedriver/chrome/browser_info.h"
 #include "chrome/test/chromedriver/chrome/devtools_client.h"
+#include "chrome/test/chromedriver/chrome/devtools_client_impl.h"
+#include "chrome/test/chromedriver/chrome/page_load_strategy.h"
 #include "chrome/test/chromedriver/chrome/status.h"
+#include "chrome/test/chromedriver/net/sync_websocket.h"
+#include "chrome/test/chromedriver/net/sync_websocket_factory.h"
+#include "chrome/test/chromedriver/net/timeout.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -248,3 +255,125 @@ TEST(ParseCallFunctionResult, ScriptError) {
   ASSERT_EQ(1, status.code());
   ASSERT_FALSE(result);
 }
+
+namespace {
+
+class MockSyncWebSocket : public SyncWebSocket {
+ public:
+  explicit MockSyncWebSocket(SyncWebSocket::StatusCode next_status)
+      : connected_(true),
+        id_(-1),
+        queued_messages_(1),
+        next_status_(next_status) {}
+  ~MockSyncWebSocket() override {}
+
+  bool IsConnected() override { return connected_; }
+
+  bool Connect(const GURL& url) override {
+    EXPECT_STREQ("http://url/", url.possibly_invalid_spec().c_str());
+    connected_ = true;
+    return true;
+  }
+
+  bool Send(const std::string& message) override { return false; }
+
+  SyncWebSocket::StatusCode ReceiveNextMessage(
+      std::string* message,
+      const Timeout& timeout) override {
+    return next_status_;
+  }
+
+  bool HasNextMessage() override { return queued_messages_ > 0; }
+
+ protected:
+  bool connected_;
+  int id_;
+  int queued_messages_;
+  SyncWebSocket::StatusCode next_status_;
+};
+
+std::unique_ptr<SyncWebSocket> CreateMockSyncWebSocket(
+    SyncWebSocket::StatusCode next_status) {
+  return std::make_unique<MockSyncWebSocket>(next_status);
+}
+
+}  // namespace
+
+TEST(CreateChild, IsNonBlocking_NoErrors) {
+  SyncWebSocketFactory factory =
+      base::Bind(&CreateMockSyncWebSocket, SyncWebSocket::kOk);
+  // CreateChild relies on client_ being a DevToolsClientImpl, so no mocking
+  std::unique_ptr<DevToolsClientImpl> client_uptr =
+      std::make_unique<DevToolsClientImpl>(factory, "http://url", "id");
+  DevToolsClientImpl* client_ptr = client_uptr.get();
+  BrowserInfo browser_info;
+  WebViewImpl parent_view(client_ptr->GetId(), true, nullptr, &browser_info,
+                          std::move(client_uptr), nullptr,
+                          PageLoadStrategy::kEager);
+  ASSERT_FALSE(parent_view.IsNonBlocking());
+
+  std::string sessionid = "2";
+  std::unique_ptr<WebViewImpl> child_view =
+      std::unique_ptr<WebViewImpl>(parent_view.CreateChild(sessionid, "1234"));
+  ASSERT_NO_FATAL_FAILURE(child_view->IsNonBlocking());
+  ASSERT_FALSE(child_view->IsNonBlocking());
+}
+
+TEST(CreateChild, Load_NoErrors) {
+  SyncWebSocketFactory factory =
+      base::Bind(&CreateMockSyncWebSocket, SyncWebSocket::kOk);
+  // CreateChild relies on client_ being a DevToolsClientImpl, so no mocking
+  std::unique_ptr<DevToolsClientImpl> client_uptr =
+      std::make_unique<DevToolsClientImpl>(factory, "http://url", "id");
+  DevToolsClientImpl* client_ptr = client_uptr.get();
+  BrowserInfo browser_info;
+  WebViewImpl parent_view(client_ptr->GetId(), true, nullptr, &browser_info,
+                          std::move(client_uptr), nullptr,
+                          PageLoadStrategy::kNone);
+  std::string sessionid = "2";
+  std::unique_ptr<WebViewImpl> child_view =
+      std::unique_ptr<WebViewImpl>(parent_view.CreateChild(sessionid, "1234"));
+
+  ASSERT_NO_FATAL_FAILURE(child_view->Load("chrome://version", nullptr));
+}
+
+TEST(CreateChild, WaitForPendingNavigations_NoErrors) {
+  SyncWebSocketFactory factory =
+      base::Bind(&CreateMockSyncWebSocket, SyncWebSocket::kTimeout);
+  // CreateChild relies on client_ being a DevToolsClientImpl, so no mocking
+  std::unique_ptr<DevToolsClientImpl> client_uptr =
+      std::make_unique<DevToolsClientImpl>(factory, "http://url", "id");
+  DevToolsClientImpl* client_ptr = client_uptr.get();
+  BrowserInfo browser_info;
+  WebViewImpl parent_view(client_ptr->GetId(), true, nullptr, &browser_info,
+                          std::move(client_uptr), nullptr,
+                          PageLoadStrategy::kNone);
+  std::string sessionid = "2";
+  std::unique_ptr<WebViewImpl> child_view =
+      std::unique_ptr<WebViewImpl>(parent_view.CreateChild(sessionid, "1234"));
+
+  // child_view gets no socket...
+  ASSERT_NO_FATAL_FAILURE(child_view->WaitForPendingNavigations(
+      "1234", Timeout(base::TimeDelta::FromMilliseconds(10)), true));
+}
+
+TEST(CreateChild, IsPendingNavigation_NoErrors) {
+  SyncWebSocketFactory factory =
+      base::Bind(&CreateMockSyncWebSocket, SyncWebSocket::kOk);
+  // CreateChild relies on client_ being a DevToolsClientImpl, so no mocking
+  std::unique_ptr<DevToolsClientImpl> client_uptr =
+      std::make_unique<DevToolsClientImpl>(factory, "http://url", "id");
+  DevToolsClientImpl* client_ptr = client_uptr.get();
+  BrowserInfo browser_info;
+  WebViewImpl parent_view(client_ptr->GetId(), true, nullptr, &browser_info,
+                          std::move(client_uptr), nullptr,
+                          PageLoadStrategy::kNormal);
+  std::string sessionid = "2";
+  std::unique_ptr<WebViewImpl> child_view =
+      std::unique_ptr<WebViewImpl>(parent_view.CreateChild(sessionid, "1234"));
+
+  Timeout timeout(base::TimeDelta::FromMilliseconds(10));
+  bool result;
+  ASSERT_NO_FATAL_FAILURE(
+      child_view->IsPendingNavigation("1234", &timeout, &result));
+}

chrome/test/chromedriver/session.cc

@@ -104,11 +104,13 @@ Status Session::GetTargetWindow(WebView** web_view) {
 
 void Session::SwitchToTopFrame() {
   frames.clear();
+  ClearNavigationState(true);
 }
 
 void Session::SwitchToParentFrame() {
   if (!frames.empty())
     frames.pop_back();
+  ClearNavigationState(false);
 }
 
 void Session::SwitchToSubFrame(const std::string& frame_id,
@@ -117,6 +119,22 @@ void Session::SwitchToSubFrame(const std::string& frame_id,
   if (!frames.empty())
     parent_frame_id = frames.back().frame_id;
   frames.push_back(FrameInfo(parent_frame_id, frame_id, chromedriver_frame_id));
+  ClearNavigationState(false);
+}
+
+void Session::ClearNavigationState(bool for_top_frame) {
+  WebView* web_view = nullptr;
+  Status status = GetTargetWindow(&web_view);
+  if (!status.IsError()) {
+    if (for_top_frame)
+      web_view->ClearNavigationState(std::string());
+    else
+      web_view->ClearNavigationState(GetCurrentFrameId());
+  } else {
+    // Do nothing; this should be very rare because callers of this function
+    // have already called GetTargetWindow.
+    // Let later code handle issues that arise from the invalid state.
+  }
 }
 
 std::string Session::GetCurrentFrameId() const {

chrome/test/chromedriver/session.h

@@ -78,6 +78,7 @@ struct Session {
   void SwitchToParentFrame();
   void SwitchToSubFrame(const std::string& frame_id,
                         const std::string& chromedriver_frame_id);
+  void ClearNavigationState(bool for_top_frame);
   std::string GetCurrentFrameId() const;
   std::vector<WebDriverLog*> GetAllLogs() const;
 

chrome/test/chromedriver/test/run_py_tests.py

@@ -2002,6 +2002,56 @@ class ChromeDriverTest(ChromeDriverBaseTestWithWebServer):
     self._driver.Load(self._http_server.GetUrl('localhost')
                       + '/chromedriver/empty.html')
 
+  def testWaitForCurrentFrameToLoad(self):
+    """Verify ChromeDriver waits for loading events of current frame
+    Regression test for bug
+    https://bugs.chromium.org/p/chromedriver/issues/detail?id=3164
+    Clicking element in frame triggers reload of that frame, click should not
+    return until loading is complete.
+    """
+    def waitAndRespond():
+      # test may not detect regression without small sleep.
+      # locally, .2 didn't fail before code change, .3 did
+      time.sleep(.5)
+      self._sync_server.RespondWithContent(
+          """
+          <html>
+            <body>
+              <p id='valueToRead'>11</p>
+            </body>
+          </html>
+          """)
+
+    self._http_server.SetDataForPath('/page10.html',
+      """
+      <html>
+        <head>
+          <title>
+            Frame
+          </title>
+          <script>
+            function reloadWith(i) {
+              window.location.assign('%s');
+            }
+          </script>
+        </head>
+        <body>
+          <button id='prev' onclick="reloadWith(9)">-1</button>
+          <button id='next' onclick="reloadWith(11)">+1</button>
+          <p id='valueToRead'>10</p>
+        </body>
+      </html>
+      """ % self._sync_server.GetUrl())
+    self._driver.Load(self.GetHttpUrlForFile(
+        '/chromedriver/page_for_next_iframe.html'))
+    frame = self._driver.FindElement('tag name', 'iframe')
+    self._driver.SwitchToFrame(frame);
+    thread = threading.Thread(target=waitAndRespond)
+    thread.start()
+    self._driver.FindElement('css selector', '#next').Click()
+    value_display = self._driver.FindElement('css selector', '#valueToRead')
+    self.assertEquals('11', value_display.GetText())
+
   def testSlowIFrame(self):
     """Verify ChromeDriver does not wait for slow frames to load.
     Regression test for bugs
@@ -2011,7 +2061,8 @@ class ChromeDriverTest(ChromeDriverBaseTestWithWebServer):
     def waitAndRespond():
       # Send iframe contents slowly
       time.sleep(2)
-      self._sync_server.RespondWithContent('<html>IFrame contents</html>')
+      self._sync_server.RespondWithContent(
+        '<html><div id=iframediv>IFrame contents</div></html>')
 
     self._http_server.SetDataForPath('/top.html',
         """
@@ -2031,15 +2082,18 @@ class ChromeDriverTest(ChromeDriverBaseTestWithWebServer):
     self._driver.Load(self._http_server.GetUrl() + '/top.html')
     thread = threading.Thread(target=waitAndRespond)
     thread.start()
+    start = time.time()
+    # Click should not wait for frame to load, so elapsed time from this
+    # command should be < 2 seconds.
     self._driver.FindElement('css selector', '#button').Click()
-    # Correct ChromeDriver behavior should not wait for iframe to
-    # load. Therefore, SwitchToFrame should fail, we remain in the top
-    # frame, and FindElement should succeed. If ChromeDriver incorrectly
-    # waits for slow iframe to load, then SwitchToFrame succeeds,
-    # and element with id='top' won't be found.
+    self.assertLess(time.time() - start, 2.0)
     frame = self._driver.FindElement('css selector', '#iframe')
+    # WaitForPendingNavigations examines the load state of the current frame
+    # so ChromeDriver will wait for frame to load after SwitchToFrame
+    # start is reused because that began the pause for the frame load
     self._driver.SwitchToFrame(frame)
-    self._driver.FindElement('css selector', '#top')
+    self.assertGreaterEqual(time.time() - start, 2.0)
+    self._driver.FindElement('css selector', '#iframediv')
     thread.join()
 
   @staticmethod

chrome/test/data/chromedriver/page_for_next_iframe.html

+<html>
+	<head>
+		<title>Test driver</title>
+	</head>
+	<body>
+		<iframe id="inlineFrameExample"
+			title="Inline Frame Example"
+			width="300"
+			height="200"
+			src="/page10.html">
+		</iframe>
+	</body>
+</html>
\ No newline at end of file