Don't silently eat Inspector.TargetCrashed

headless/app/headless_shell.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <iostream>
 #include <memory>
 #include <string>
 
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/json/json_writer.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/string_number_conversions.h"
 #include "content/public/common/content_switches.h"
 #include "headless/app/headless_shell_switches.h"
 #include "headless/public/devtools/domains/emulation.h"
+#include "headless/public/devtools/domains/inspector.h"
 #include "headless/public/devtools/domains/page.h"
 #include "headless/public/devtools/domains/runtime.h"
 #include "headless/public/headless_browser.h"
 #include "headless/public/headless_devtools_client.h"
 #include "headless/public/headless_devtools_target.h"
 #include "headless/public/headless_web_contents.h"
 #include "headless/public/util/deterministic_dispatcher.h"
 #include "headless/public/util/deterministic_http_protocol_handler.h"
 #include "net/base/file_stream.h"
 #include "net/base/io_buffer.h"
@@ skipping 16 matching lines @@
     parsed_window_size->set_height(height);
     return true;
   }
   return false;
 }
 }  // namespace
 
 // An application which implements a simple headless browser.
 class HeadlessShell : public HeadlessWebContents::Observer,
                       emulation::ExperimentalObserver,
+                      inspector::ExperimentalObserver,
                       page::Observer {
  public:
   HeadlessShell()
       : browser_(nullptr),
         devtools_client_(HeadlessDevToolsClient::Create()),
         web_contents_(nullptr),
         processed_page_ready_(false),
         browser_context_(nullptr) {}
   ~HeadlessShell() override {}
 
@@ skipping 43 matching lines @@
       return;
     }
     web_contents_->AddObserver(this);
   }
 
   void Shutdown() {
     if (!web_contents_)
       return;
     if (!RemoteDebuggingEnabled()) {
       devtools_client_->GetEmulation()->GetExperimental()->RemoveObserver(this);
+      devtools_client_->GetInspector()->GetExperimental()->RemoveObserver(this);
       devtools_client_->GetPage()->RemoveObserver(this);
       if (web_contents_->GetDevToolsTarget()) {
         web_contents_->GetDevToolsTarget()->DetachClient(
             devtools_client_.get());
       }
     }
     web_contents_->RemoveObserver(this);
     web_contents_ = nullptr;
     browser_context_->Close();
     browser_->Shutdown();
   }
 
   // HeadlessWebContents::Observer implementation:
   void DevToolsTargetReady() override {
     if (RemoteDebuggingEnabled())
       return;
     web_contents_->GetDevToolsTarget()->AttachClient(devtools_client_.get());
+    devtools_client_->GetInspector()->GetExperimental()->AddObserver(this);
     devtools_client_->GetPage()->AddObserver(this);
     devtools_client_->GetPage()->Enable();
     // Check if the document had already finished loading by the time we
     // attached.
 
     devtools_client_->GetEmulation()->GetExperimental()->AddObserver(this);
 
     if (base::CommandLine::ForCurrentProcess()->HasSwitch(
             switches::kVirtualTimeBudget)) {
       std::string budget_ms_ascii =
           base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
               switches::kVirtualTimeBudget);
       int budget_ms;
       CHECK(base::StringToInt(budget_ms_ascii, &budget_ms))
           << "Expected an integer value for --virtual-time-budget=";
       devtools_client_->GetEmulation()->GetExperimental()->SetVirtualTimePolicy(
           emulation::SetVirtualTimePolicyParams::Builder()
               .SetPolicy(emulation::VirtualTimePolicy::
                              PAUSE_IF_NETWORK_FETCHES_PENDING)
               .SetBudget(budget_ms)
               .Build());
     } else {
       PollReadyState();
     }
     // TODO(skyostil): Implement more features to demonstrate the devtools API.
   }
 
-  void RenderProcessExited(base::TerminationStatus status,
-                           int exit_code) override {
-    if (status == base::TERMINATION_STATUS_NORMAL_TERMINATION)
-      return;
-
+  void OnTargetCrashed(const inspector::TargetCrashedParams& params) override {
     LOG(ERROR) << "Abnormal renderer termination.";
     Shutdown();

[Sami, 2016/11/21 14:59:28]

Should we remove this call to Shutdown() too (could be a TODO too)? If someone
wants to use Headless Shell as a long running rendering backend, tearing it down
if one renderer crashes isn't ideal.

[alex clarke (OOO till 29th), 2016/11/22 10:48:55]

Do people do that?

tearing it down
Well people where complaining that it never dumped the dom or generated a screen
shot.

[Sami, 2016/11/22 11:23:11]

Yeah, I would expect the NodeJS module to be used as a long running server.
That's how I think the existing Xvfb-version is used. There's a lot of overhead
in spinning up a new browser for every request.

I think we should come up with some guideline about what role Headless Shell
should fill. In my mind it should be less of a headless swiss army knife (we can
write a Node front-end which is that instead) and more of a dumb programmable
browser. Right now it's sort of an awkward mix of the two -- and for sort of a
good reason since we intended it to be a demo app. Now that --headless is going
to productize it, I think we should rethink a bit.

Maybe for the near term we could decide on a split where convenience stuff like
this automatic shutdown only happens if you're not using remote debugging. WDYT?

[alex clarke (OOO till 29th), 2016/11/22 15:30:58]

Note due to the way DevToolsTargetReady is written the observer never gets
registered if remote debugging is enabled.  I added a comment.

   }
 
   void PollReadyState() {
     // We need to check the current location in addition to the ready state to
     // be sure the expected page is ready.
     devtools_client_->GetRuntime()->Evaluate(
         "document.readyState + ' ' + document.location.href",
         base::Bind(&HeadlessShell::OnReadyState, base::Unretained(this)));
   }
 
@@ skipping 259 matching lines @@
     }
     builder.SetWindowSize(parsed_window_size);
   }
 
   return HeadlessBrowserMain(
       builder.Build(),
       base::Bind(&HeadlessShell::OnStart, base::Unretained(&shell)));
 }
 
 }  // namespace headless