[ChromeDriver] Fix race condition in frame detach

Fix another race condition in frame detach sequence, found by stress
testing.

Change-Id: Ie342733a8627bfd36074d38b73209c751ba519dc
Reviewed-on: https://chromium-review.googlesource.com/1014506Reviewed-by: Jonathon Kereliuk <kereliuk@chromium.org>
Commit-Queue: John Chen <johnchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551350}

chrome/test/chromedriver/chrome/devtools_client.h

@@ -17,6 +17,7 @@ class DictionaryValue;
 class DevToolsEventListener;
 class Timeout;
 class Status;
+class WebViewImpl;
 
 // A DevTools client of a single DevTools debugger.
 class DevToolsClient {
@@ -76,6 +77,9 @@ class DevToolsClient {
 
   // Indicate that we've been detached from the DevTools target.
   virtual void SetDetached() = 0;
+
+  // Set the owning WebViewImpl, if any.
+  virtual void SetOwner(WebViewImpl* owner) = 0;
 };
 
 #endif  // CHROME_TEST_CHROMEDRIVER_CHROME_DEVTOOLS_CLIENT_H_

chrome/test/chromedriver/chrome/devtools_client_impl.cc

@@ -16,6 +16,7 @@
 #include "chrome/test/chromedriver/chrome/log.h"
 #include "chrome/test/chromedriver/chrome/status.h"
 #include "chrome/test/chromedriver/chrome/util.h"
+#include "chrome/test/chromedriver/chrome/web_view_impl.h"
 #include "chrome/test/chromedriver/net/sync_websocket.h"
 #include "chrome/test/chromedriver/net/url_request_context_getter.h"
 
@@ -89,6 +90,7 @@ DevToolsClientImpl::DevToolsClientImpl(const SyncWebSocketFactory& factory,
     : socket_(factory.Run()),
       url_(url),
       parent_(nullptr),
+      owner_(nullptr),
       crashed_(false),
       detached_(false),
       id_(id),
@@ -106,6 +108,7 @@ DevToolsClientImpl::DevToolsClientImpl(
     : socket_(factory.Run()),
       url_(url),
       parent_(nullptr),
+      owner_(nullptr),
       crashed_(false),
       detached_(false),
       id_(id),
@@ -118,6 +121,7 @@ DevToolsClientImpl::DevToolsClientImpl(
 DevToolsClientImpl::DevToolsClientImpl(DevToolsClientImpl* parent,
                                        const std::string& session_id)
     : parent_(parent),
+      owner_(nullptr),
       session_id_(session_id),
       crashed_(false),
       detached_(false),
@@ -139,6 +143,7 @@ DevToolsClientImpl::DevToolsClientImpl(
     : socket_(factory.Run()),
       url_(url),
       parent_(nullptr),
+      owner_(nullptr),
       crashed_(false),
       detached_(false),
       id_(id),
@@ -279,6 +284,10 @@ void DevToolsClientImpl::SetDetached() {
   detached_ = true;
 }
 
+void DevToolsClientImpl::SetOwner(WebViewImpl* owner) {
+  owner_ = owner;
+}
+
 DevToolsClientImpl::ResponseInfo::ResponseInfo(const std::string& method)
     : state(kWaiting), method(method) {}
 
@@ -481,6 +490,7 @@ Status DevToolsClientImpl::ProcessEvent(const internal::InspectorEvent& event) {
           kUnknownError,
           "missing message in Target.receivedMessageFromTarget event");
 
+    WebViewImplHolder childHolder(child->owner_);
     return child->HandleMessage(-1, message);
   }
   return Status(kOk);

chrome/test/chromedriver/chrome/devtools_client_impl.h

@@ -114,6 +114,7 @@ class DevToolsClientImpl : public DevToolsClient {
                            const Timeout& timeout) override;
   Status HandleReceivedEvents() override;
   void SetDetached() override;
+  void SetOwner(WebViewImpl* owner) override;
 
  private:
   enum ResponseState {
@@ -157,6 +158,8 @@ class DevToolsClientImpl : public DevToolsClient {
   std::unique_ptr<SyncWebSocket> socket_;
   GURL url_;
   DevToolsClientImpl* parent_;
+  // WebViewImpl that owns this instance; nullptr for browser-wide DevTools.
+  WebViewImpl* owner_;
   const std::string session_id_;
   std::map<std::string, DevToolsClientImpl*> children_;
   bool crashed_;

chrome/test/chromedriver/chrome/stub_devtools_client.cc

@@ -82,3 +82,5 @@ Status StubDevToolsClient::HandleReceivedEvents() {
 }
 
 void StubDevToolsClient::SetDetached() {}
+
+void StubDevToolsClient::SetOwner(WebViewImpl* owner) {}

chrome/test/chromedriver/chrome/stub_devtools_client.h

@@ -55,6 +55,7 @@ class StubDevToolsClient : public DevToolsClient {
                            const Timeout& timeout) override;
   Status HandleReceivedEvents() override;
   void SetDetached() override;
+  void SetOwner(WebViewImpl* owner) override;
 
  protected:
   const std::string id_;

chrome/test/chromedriver/chrome/web_view_impl.cc

@@ -147,7 +147,9 @@ WebViewImpl::WebViewImpl(const std::string& id,
       network_conditions_override_manager_(
           new NetworkConditionsOverrideManager(client_.get())),
       heap_snapshot_taker_(new HeapSnapshotTaker(client_.get())),
-      debugger_(new DebuggerTracker(client_.get())) {}
+      debugger_(new DebuggerTracker(client_.get())) {
+  client_->SetOwner(this);
+}
 
 WebViewImpl::~WebViewImpl() {}
 

chrome/test/chromedriver/chrome/web_view_impl_unittest.cc

@@ -77,6 +77,7 @@ class FakeDevToolsClient : public DevToolsClient {
   }
   Status HandleReceivedEvents() override { return Status(kOk); }
   void SetDetached() override {}
+  void SetOwner(WebViewImpl* owner) override {}
 
  private:
   const std::string id_;