headless: Require the user to pass in an initial URL

Require the user to pass in an initial URL when creating a
HeadlessWebContents. This is to ensure that we have a navigation that
initializes the renderer, making it possible to inspect the tab.

BUG=595353,546953

Review URL: https://codereview.chromium.org/1858403003

Cr-Commit-Position: refs/heads/master@{#385476}

headless/app/headless_shell.cc

@@ -37,8 +37,6 @@ class HeadlessShell : public HeadlessWebContents::Observer {
 
   void OnStart(HeadlessBrowser* browser) {
     browser_ = browser;
-    web_contents_ = browser->CreateWebContents(gfx::Size(800, 600));
-    web_contents_->AddObserver(this);
 
     base::CommandLine::StringVector args =
         base::CommandLine::ForCurrentProcess()->GetArgs();
@@ -50,11 +48,13 @@ class HeadlessShell : public HeadlessWebContents::Observer {
     } else {
       url = GURL(args[0]);
     }
-    if (!web_contents_->OpenURL(url)) {
+    web_contents_ = browser->CreateWebContents(url, gfx::Size(800, 600));
+    if (!web_contents_) {
       LOG(ERROR) << "Navigation failed";
-      web_contents_ = nullptr;
       browser_->Shutdown();
+      return;
     }
+    web_contents_->AddObserver(this);
   }
 
   void ShutdownIfNeeded() {
@@ -71,8 +71,6 @@ class HeadlessShell : public HeadlessWebContents::Observer {
     ShutdownIfNeeded();
   }
 
-  void DidFinishNavigation(bool success) override {}
-
  private:
   HeadlessBrowser* browser_;  // Not owned.
   std::unique_ptr<HeadlessWebContents> web_contents_;

headless/lib/browser/headless_browser_impl.cc

@@ -31,10 +31,18 @@ HeadlessBrowserImpl::HeadlessBrowserImpl(
 HeadlessBrowserImpl::~HeadlessBrowserImpl() {}
 
 std::unique_ptr<HeadlessWebContents> HeadlessBrowserImpl::CreateWebContents(
+    const GURL& initial_url,
     const gfx::Size& size) {
   DCHECK(BrowserMainThread()->BelongsToCurrentThread());
-  return base::WrapUnique(new HeadlessWebContentsImpl(
-      browser_context(), window_tree_host_->window(), size));
+  std::unique_ptr<HeadlessWebContentsImpl> web_contents =
+      base::WrapUnique(new HeadlessWebContentsImpl(
+          browser_context(), window_tree_host_->window(), size));
+  // We require the user to pass in an initial URL to ensure that the renderer
+  // gets initialized and eventually becomes ready to be inspected. See
+  // HeadlessWebContents::Observer::WebContentsReady.
+  if (!web_contents->OpenURL(initial_url))
+    return nullptr;
+  return std::move(web_contents);
 }
 
 scoped_refptr<base::SingleThreadTaskRunner>

headless/lib/browser/headless_browser_impl.h

@@ -30,6 +30,7 @@ class HeadlessBrowserImpl : public HeadlessBrowser {
 
   // HeadlessBrowser implementation:
   std::unique_ptr<HeadlessWebContents> CreateWebContents(
+      const GURL& initial_url,
       const gfx::Size& size) override;
   scoped_refptr<base::SingleThreadTaskRunner> BrowserMainThread()
       const override;

headless/lib/browser/headless_web_contents_impl.cc

@@ -31,6 +31,8 @@ class WebContentsObserverAdapter : public content::WebContentsObserver {
 
   ~WebContentsObserverAdapter() override {}
 
+  void RenderViewReady() override { observer_->WebContentsReady(); }
+
   void DocumentOnLoadCompletedInMainFrame() override {
     observer_->DocumentOnLoadCompletedInMainFrame();
   }

headless/lib/browser/headless_web_contents_impl.h

@@ -28,22 +28,19 @@ class WebContentsObserverAdapter;
 
 class HeadlessWebContentsImpl : public HeadlessWebContents {
  public:
+  HeadlessWebContentsImpl(content::BrowserContext* context,
+                          aura::Window* parent_window,
+                          const gfx::Size& initial_size);
   ~HeadlessWebContentsImpl() override;
 
   // HeadlessWebContents implementation:
-  bool OpenURL(const GURL& url) override;
   void AddObserver(Observer* observer) override;
   void RemoveObserver(Observer* observer) override;
 
   content::WebContents* web_contents() const;
+  bool OpenURL(const GURL& url);
 
  private:
-  friend class HeadlessBrowserImpl;
-
-  HeadlessWebContentsImpl(content::BrowserContext* context,
-                          aura::Window* parent_window,
-                          const gfx::Size& initial_size);
-
   class Delegate;
   std::unique_ptr<Delegate> web_contents_delegate_;
   std::unique_ptr<content::WebContents> web_contents_;

headless/lib/headless_browser_browsertest.cc

@@ -16,12 +16,19 @@ namespace headless {
 
 IN_PROC_BROWSER_TEST_F(HeadlessBrowserTest, CreateAndDestroyWebContents) {
   std::unique_ptr<HeadlessWebContents> web_contents =
-      browser()->CreateWebContents(gfx::Size(800, 600));
+      browser()->CreateWebContents(GURL("about:blank"), gfx::Size(800, 600));
   EXPECT_TRUE(web_contents);
   // TODO(skyostil): Verify viewport dimensions once we can.
   web_contents.reset();
 }
 
+IN_PROC_BROWSER_TEST_F(HeadlessBrowserTest, CreateWithBadURL) {
+  GURL bad_url("not_valid");
+  std::unique_ptr<HeadlessWebContents> web_contents =
+      browser()->CreateWebContents(bad_url, gfx::Size(800, 600));
+  EXPECT_FALSE(web_contents);
+}
+
 class HeadlessBrowserTestWithProxy : public HeadlessBrowserTest {
  public:
   HeadlessBrowserTestWithProxy()
@@ -51,13 +58,13 @@ IN_PROC_BROWSER_TEST_F(HeadlessBrowserTestWithProxy, SetProxyServer) {
   builder.SetProxyServer(proxy_server()->host_port_pair());
   SetBrowserOptions(builder.Build());
 
-  std::unique_ptr<HeadlessWebContents> web_contents =
-      browser()->CreateWebContents(gfx::Size(800, 600));
-
   // Load a page which doesn't actually exist, but for which the our proxy
   // returns valid content anyway.
-  EXPECT_TRUE(NavigateAndWaitForLoad(
-      web_contents.get(), GURL("http://not-an-actual-domain.tld/hello.html")));
+  std::unique_ptr<HeadlessWebContents> web_contents =
+      browser()->CreateWebContents(
+          GURL("http://not-an-actual-domain.tld/hello.html"),
+          gfx::Size(800, 600));
+  EXPECT_TRUE(WaitForLoad(web_contents.get()));
 }
 
 }  // namespace headless

headless/lib/headless_web_contents_browsertest.cc

@@ -16,19 +16,39 @@ namespace headless {
 
 class HeadlessWebContentsTest : public HeadlessBrowserTest {};
 
+class NavigationObserver : public HeadlessWebContents::Observer {
+ public:
+  NavigationObserver(HeadlessWebContentsTest* browser_test)
+      : browser_test_(browser_test), navigation_succeeded_(false) {}
+  ~NavigationObserver() override {}
+
+  void DocumentOnLoadCompletedInMainFrame() override {
+    browser_test_->FinishAsynchronousTest();
+  }
+
+  void DidFinishNavigation(bool success) override {
+    navigation_succeeded_ = success;
+  }
+
+  bool navigation_succeeded() const { return navigation_succeeded_; }
+
+ private:
+  HeadlessWebContentsTest* browser_test_;  // Not owned.
+  bool navigation_succeeded_;
+};
+
 IN_PROC_BROWSER_TEST_F(HeadlessWebContentsTest, Navigation) {
   EXPECT_TRUE(embedded_test_server()->Start());
   std::unique_ptr<HeadlessWebContents> web_contents =
-      browser()->CreateWebContents(gfx::Size(800, 600));
-  EXPECT_TRUE(NavigateAndWaitForLoad(
-      web_contents.get(), embedded_test_server()->GetURL("/hello.html")));
-}
+      browser()->CreateWebContents(
+          embedded_test_server()->GetURL("/hello.html"), gfx::Size(800, 600));
+  NavigationObserver observer(this);
+  web_contents->AddObserver(&observer);
 
-IN_PROC_BROWSER_TEST_F(HeadlessWebContentsTest, NavigationWithBadURL) {
-  std::unique_ptr<HeadlessWebContents> web_contents =
-      browser()->CreateWebContents(gfx::Size(800, 600));
-  GURL bad_url("not_valid");
-  EXPECT_FALSE(web_contents->OpenURL(bad_url));
+  RunAsynchronousTest();
+
+  EXPECT_TRUE(observer.navigation_succeeded());
+  web_contents->RemoveObserver(&observer);
 }
 
 }  // namespace headless

headless/public/headless_browser.h

@@ -35,8 +35,10 @@ class HEADLESS_EXPORT HeadlessBrowser {
  public:
   struct Options;
 
-  // Create a new browser tab. |size| is in physical pixels.
+  // Create a new browser tab which navigates to |initial_url|. |size| is in
+  // physical pixels.
   virtual std::unique_ptr<HeadlessWebContents> CreateWebContents(
+      const GURL& initial_url,
       const gfx::Size& size) = 0;
 
   // Returns a task runner for submitting work to the browser main thread.

headless/public/headless_web_contents.h

@@ -22,9 +22,13 @@ class HEADLESS_EXPORT HeadlessWebContents {
   // TODO(skyostil): Replace this with an equivalent client API.
   class Observer {
    public:
-    // Will be called on browser thread.
-    virtual void DocumentOnLoadCompletedInMainFrame() = 0;
-    virtual void DidFinishNavigation(bool success) = 0;
+    // All the following notifications will be called on browser main thread.
+    virtual void DocumentOnLoadCompletedInMainFrame(){};
+    virtual void DidFinishNavigation(bool success){};
+
+    // After this event, this HeadlessWebContents instance is ready to be
+    // controlled using a DevTools client.
+    virtual void WebContentsReady(){};
 
    protected:
     Observer() {}
@@ -34,9 +38,6 @@ class HEADLESS_EXPORT HeadlessWebContents {
     DISALLOW_COPY_AND_ASSIGN(Observer);
   };
 
-  // TODO(skyostil): Replace this with an equivalent client API.
-  virtual bool OpenURL(const GURL& url) = 0;
-
   // Add or remove an observer to receive events from this WebContents.
   // |observer| must outlive this class or be removed prior to being destroyed.
   virtual void AddObserver(Observer* observer) = 0;

headless/test/headless_browser_test.cc

@@ -5,6 +5,7 @@
 #include "headless/test/headless_browser_test.h"
 
 #include "base/files/file_path.h"
+#include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "content/public/browser/browser_thread.h"
@@ -18,25 +19,27 @@ namespace {
 
 class WaitForNavigationObserver : public HeadlessWebContents::Observer {
  public:
-  WaitForNavigationObserver(base::RunLoop* run_loop,
+  WaitForNavigationObserver(HeadlessBrowserTest* browser_test,
                             HeadlessWebContents* web_contents)
-      : run_loop_(run_loop),
+      : browser_test_(browser_test),
         web_contents_(web_contents),
         navigation_succeeded_(false) {
     web_contents_->AddObserver(this);
   }
 
   ~WaitForNavigationObserver() override { web_contents_->RemoveObserver(this); }
 
-  void DocumentOnLoadCompletedInMainFrame() override { run_loop_->Quit(); }
+  void DocumentOnLoadCompletedInMainFrame() override {
+    browser_test_->FinishAsynchronousTest();
+  }
   void DidFinishNavigation(bool success) override {
     navigation_succeeded_ = success;
   }
 
   bool navigation_succeeded() const { return navigation_succeeded_; }
 
  private:
-  base::RunLoop* run_loop_;            // Not owned.
+  HeadlessBrowserTest* browser_test_;  // Not owned.
   HeadlessWebContents* web_contents_;  // Not owned.
 
   bool navigation_succeeded_;
@@ -86,18 +89,22 @@ HeadlessBrowser* HeadlessBrowserTest::browser() const {
   return HeadlessContentMainDelegate::GetInstance()->browser();
 }
 
-bool HeadlessBrowserTest::NavigateAndWaitForLoad(
-    HeadlessWebContents* web_contents,
-    const GURL& url) {
-  base::RunLoop run_loop;
+bool HeadlessBrowserTest::WaitForLoad(HeadlessWebContents* web_contents) {
+  WaitForNavigationObserver observer(this, web_contents);
+  RunAsynchronousTest();
+  return observer.navigation_succeeded();
+}
+
+void HeadlessBrowserTest::RunAsynchronousTest() {
   base::MessageLoop::ScopedNestableTaskAllower nestable_allower(
       base::MessageLoop::current());
-  WaitForNavigationObserver observer(&run_loop, web_contents);
+  run_loop_ = base::WrapUnique(new base::RunLoop());
+  run_loop_->Run();
+  run_loop_ = nullptr;
+}
 
-  if (!web_contents->OpenURL(url))
-    return false;
-  run_loop.Run();
-  return observer.navigation_succeeded();
+void HeadlessBrowserTest::FinishAsynchronousTest() {
+  run_loop_->Quit();
 }
 
 }  // namespace headless

headless/test/headless_browser_test.h

@@ -5,14 +5,24 @@
 #ifndef HEADLESS_TEST_HEADLESS_BROWSER_TEST_H_
 #define HEADLESS_TEST_HEADLESS_BROWSER_TEST_H_
 
+#include <memory>
 #include "content/public/test/browser_test_base.h"
 #include "headless/public/headless_browser.h"
 
+namespace base {
+class RunLoop;
+}
+
 namespace headless {
 class HeadlessWebContents;
 
 // Base class for tests which require a full instance of the headless browser.
 class HeadlessBrowserTest : public content::BrowserTestBase {
+ public:
+  // Notify that an asynchronous test is now complete and the test runner should
+  // exit.
+  void FinishAsynchronousTest();
+
  protected:
   HeadlessBrowserTest();
   ~HeadlessBrowserTest() override;
@@ -27,15 +37,20 @@ class HeadlessBrowserTest : public content::BrowserTestBase {
   // pumps) cannot be set via this method.
   void SetBrowserOptions(const HeadlessBrowser::Options& options);
 
-  // Navigate to |url| and wait for the document load to complete.
-  bool NavigateAndWaitForLoad(HeadlessWebContents* web_contents,
-                              const GURL& url);
+  // Run an asynchronous test in a nested run loop. The caller should call
+  // FinishAsynchronousTest() to notify that the test should finish.
+  void RunAsynchronousTest();
+
+  // Synchronously waits for a tab to finish loading.
+  bool WaitForLoad(HeadlessWebContents* web_contents);
 
  protected:
   // Returns the browser for the test.
   HeadlessBrowser* browser() const;
 
  private:
+  std::unique_ptr<base::RunLoop> run_loop_;
+
   DISALLOW_COPY_AND_ASSIGN(HeadlessBrowserTest);
 };