Properly handle HeadlessWebContents being closed as a result of page actions

Detach HeadlessShell from HeadlessWebContents when the latter is being closed.

Bug: 958755
Change-Id: Ief08c630bb3b558265be31e7ff9e6a2566b594f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1593829
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Johannes Henkel <johannes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#656967}

headless/app/headless_shell.cc

@@ -251,9 +251,7 @@ void HeadlessShell::OnGotURLs(const std::vector<GURL>& urls) {
   }
 }
 
-void HeadlessShell::Shutdown() {
-  if (!web_contents_)
-    return;
+void HeadlessShell::Detach() {
   if (!RemoteDebuggingEnabled()) {
     devtools_client_->GetEmulation()->GetExperimental()->RemoveObserver(this);
     devtools_client_->GetInspector()->GetExperimental()->RemoveObserver(this);
@@ -264,6 +262,11 @@ void HeadlessShell::Shutdown() {
   }
   web_contents_->RemoveObserver(this);
   web_contents_ = nullptr;
+}
+
+void HeadlessShell::Shutdown() {
+  if (web_contents_)
+    Detach();
   browser_context_->Close();
   browser_->Shutdown();
 }
@@ -332,9 +335,17 @@ void HeadlessShell::DevToolsTargetReady() {
                        weak_factory_.GetWeakPtr()),
         base::TimeDelta::FromMilliseconds(timeout_ms));
   }
-
   // TODO(skyostil): Implement more features to demonstrate the devtools API.
 }
+
+void HeadlessShell::HeadlessWebContentsDestroyed() {
+  // Detach now, but defer shutdown till the HeadlessWebContents
+  // removal is complete.
+  Detach();
+  browser_->BrowserMainThread()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&HeadlessShell::Shutdown, weak_factory_.GetWeakPtr()));
+}
 #endif  // !defined(CHROME_MULTIPLE_DLL_CHILD)
 
 void HeadlessShell::FetchTimeout() {

headless/app/headless_shell.h

@@ -33,7 +33,7 @@ class HeadlessShell : public HeadlessWebContents::Observer,
   HeadlessShell();
   ~HeadlessShell() override;
 
-  virtual void OnStart(HeadlessBrowser* browser);
+  void OnStart(HeadlessBrowser* browser);
 
   HeadlessDevToolsClient* devtools_client() const {
     return devtools_client_.get();
@@ -43,6 +43,7 @@ class HeadlessShell : public HeadlessWebContents::Observer,
   // HeadlessWebContents::Observer implementation:
   void DevToolsTargetReady() override;
   void OnTargetCrashed(const inspector::TargetCrashedParams& params) override;
+  void HeadlessWebContentsDestroyed() override;
 
   // emulation::Observer implementation:
   void OnVirtualTimeBudgetExpired(
@@ -51,7 +52,8 @@ class HeadlessShell : public HeadlessWebContents::Observer,
   // page::Observer implementation:
   void OnLoadEventFired(const page::LoadEventFiredParams& params) override;
 
-  virtual void Shutdown();
+  void Detach();
+  void Shutdown();
 
   void FetchTimeout();
 

headless/app/headless_shell_win.cc

@@ -14,6 +14,8 @@ void HeadlessShell::OnStart(HeadlessBrowser* browser) {}
 void HeadlessShell::Shutdown() {}
 
 void HeadlessShell::DevToolsTargetReady() {}
+
+void HeadlessShell::HeadlessWebContentsDestroyed() {}
 #endif  // defined(CHROME_MULTIPLE_DLL_CHILD)
 
 }  // namespace headless

headless/lib/browser/headless_web_contents_impl.cc

@@ -306,6 +306,8 @@ HeadlessWebContentsImpl::HeadlessWebContentsImpl(
 }
 
 HeadlessWebContentsImpl::~HeadlessWebContentsImpl() {
+  for (auto& observer : observers_)
+    observer.HeadlessWebContentsDestroyed();
   agent_host_->RemoveObserver(this);
   if (render_process_host_)
     render_process_host_->RemoveObserver(this);

headless/public/headless_web_contents.h

@@ -59,6 +59,9 @@ class HEADLESS_EXPORT HeadlessWebContents {
     virtual void RenderProcessExited(base::TerminationStatus status,
                                      int exit_code) {}
 
+    // Invoked when HeadlessWebContents is being destroyed.
+    virtual void HeadlessWebContentsDestroyed() {}
+
    protected:
     Observer() {}
     virtual ~Observer() {}