[ChromeDriver] Detect stale frame during navigation

While waiting for a pending navigation, verify that the current
frame still exists. Otherwise we might wait forever for a frame
that has already been destroyed.

Bug: chromedriver:3353
Change-Id: I0e80631a3880d24bf207e0871db07f31dad8100b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2129088
Commit-Queue: John Chen <johnchen@chromium.org>
Reviewed-by: Wenbin Zhang <wenbinzhang@google.com>
Cr-Commit-Position: refs/heads/master@{#755133}

chrome/test/chromedriver/chrome/frame_tracker.cc

@@ -52,6 +52,21 @@ WebView* FrameTracker::GetTargetForFrame(const std::string& frame_id) {
   return nullptr;
 }
 
+bool FrameTracker::IsKnownFrame(const std::string& frame_id) const {
+  if (attached_frames_.count(frame_id) != 0 ||
+      frame_to_context_map_.count(frame_id) != 0 ||
+      frame_to_target_map_.count(frame_id) != 0) {
+    return true;
+  }
+  // Frame unknown to this tracker, recursively search all child targets.
+  for (const auto& it : frame_to_target_map_) {
+    FrameTracker* child = it.second->GetFrameTracker();
+    if (child != nullptr && child->IsKnownFrame(frame_id))
+      return true;
+  }
+  return false;
+}
+
 void FrameTracker::DeleteTargetForFrame(const std::string& frame_id) {
   frame_to_target_map_.erase(frame_id);
 }
@@ -59,6 +74,7 @@ void FrameTracker::DeleteTargetForFrame(const std::string& frame_id) {
 Status FrameTracker::OnConnected(DevToolsClient* client) {
   frame_to_context_map_.clear();
   frame_to_target_map_.clear();
+  attached_frames_.clear();
   // Enable target events to allow tracking iframe targets creation.
   base::DictionaryValue params;
   params.SetBoolean("autoAttach", true);
@@ -141,6 +157,18 @@ Status FrameTracker::OnEvent(DevToolsClient* client,
     }
   } else if (method == "Runtime.executionContextsCleared") {
     frame_to_context_map_.clear();
+  } else if (method == "Page.frameAttached") {
+    std::string frame_id;
+    if (!params.GetString("frameId", &frame_id))
+      return Status(kUnknownError,
+                    "missing frameId in Page.frameAttached event");
+    attached_frames_.insert(frame_id);
+  } else if (method == "Page.frameDetached") {
+    std::string frame_id;
+    if (!params.GetString("frameId", &frame_id))
+      return Status(kUnknownError,
+                    "missing frameId in Page.frameDetached event");
+    attached_frames_.erase(frame_id);
   } else if (method == "Page.frameNavigated") {
     const base::Value* unused_value;
     if (!params.Get("frame.parentId", &unused_value))

chrome/test/chromedriver/chrome/frame_tracker.h

@@ -7,6 +7,7 @@
 
 #include <map>
 #include <string>
+#include <unordered_set>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
@@ -31,6 +32,7 @@ class FrameTracker : public DevToolsEventListener {
 
   Status GetContextIdForFrame(const std::string& frame_id, int* context_id);
   WebView* GetTargetForFrame(const std::string& frame_id);
+  bool IsKnownFrame(const std::string& frame_id) const;
   void DeleteTargetForFrame(const std::string& frame_id);
 
   // Overridden from DevToolsEventListener:
@@ -42,6 +44,7 @@ class FrameTracker : public DevToolsEventListener {
  private:
   std::map<std::string, int> frame_to_context_map_;
   std::map<std::string, std::unique_ptr<WebView>> frame_to_target_map_;
+  std::unordered_set<std::string> attached_frames_;
   WebView* web_view_;
 
   DISALLOW_COPY_AND_ASSIGN(FrameTracker);

chrome/test/chromedriver/chrome/web_view_impl.cc

@@ -1125,6 +1125,11 @@ void WebViewImpl::ClearNavigationState(const std::string& new_frame_id) {
 Status WebViewImpl::IsNotPendingNavigation(const std::string& frame_id,
                                            const Timeout* timeout,
                                            bool* is_not_pending) {
+  if (!frame_id.empty() && !frame_tracker_->IsKnownFrame(frame_id)) {
+    // Frame has already been destroyed.
+    *is_not_pending = true;
+    return Status(kOk);
+  }
   bool is_pending;
   Status status =
       navigation_tracker_->IsPendingNavigation(frame_id, timeout, &is_pending);