Add support for distilling current WebContents

components/dom_distiller/content/distiller_page_web_contents_browsertest.cc

Index: components/dom_distiller/content/distiller_page_web_contents_browsertest.cc
diff --git a/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc b/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc
index 72b572f4ff3ac1aed703e71f1193757bdaf8c339..0e7226d593b82087c345577a86c255f9a46e7765 100644
--- a/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc
+++ b/components/dom_distiller/content/distiller_page_web_contents_browsertest.cc
@@ -7,8 +7,12 @@
 #include "base/run_loop.h"
 #include "base/values.h"
 #include "components/dom_distiller/content/distiller_page_web_contents.h"
+#include "components/dom_distiller/content/web_contents_main_frame_observer.h"
 #include "components/dom_distiller/core/distiller_page.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/navigation_controller.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/shell/browser/shell.h"
 #include "grit/component_resources.h"
@@ -23,6 +27,8 @@ using testing::Not;
 
 namespace dom_distiller {
 
+const char* kSimpleArticlePath = "/simple_article.html";
+
 class DistillerPageWebContentsTest : public ContentBrowserTest {
  public:
   // ContentBrowserTest:
@@ -65,18 +71,110 @@ class DistillerPageWebContentsTest : public ContentBrowserTest {
   }
 
  protected:
+  void RunUseCurrentWebContentsTest(const std::string& url,
+                                    bool use_new_web_contents,
+                                    bool setup_main_frame_observer,
+                                    bool wait_for_document_loaded);
+
   DistillerPageWebContents* distiller_page_;
   base::Closure quit_closure_;
   scoped_ptr<DistilledPageInfo> page_info_;
 };
 
+// Use this class to be able to leak the WebContents, which is needed for when
+// the current WebContents is used for distillation.
+class TestDistillerPageWebContents : public DistillerPageWebContents {
+ public:
+  TestDistillerPageWebContents(
+      content::BrowserContext* browser_context,
+      scoped_ptr<SourcePageHandleWebContents> optional_web_contents_handle,
+      bool use_new_web_contents)
+      : DistillerPageWebContents(browser_context,
+                                 optional_web_contents_handle.Pass()),
+        use_new_web_contents_(use_new_web_contents),
+        new_web_contents_created_(false) {}
+
+  virtual void CreateNewWebContents(const GURL& url) OVERRIDE {
+    EXPECT_EQ(true, use_new_web_contents_);

[Yaron, 2014/05/22 18:25:02]

ASSERT_EQ? I think a lot's going to go south if you miss this assertion.

[nyquist, 2014/05/22 23:00:54]

Done.

+    new_web_contents_created_ = true;
+    if (use_new_web_contents_) {

[Yaron, 2014/05/22 18:25:02]

remove this

[nyquist, 2014/05/22 23:00:54]

Done.

+      // DistillerPageWebContents::CreateNewWebContents resets the scoped_ptr to
+      // the WebContents, so intentionally leak WebContents here, since it is
+      // owned by the shell.
+      content::WebContents* web_contents = web_contents_.release();
+      web_contents->GetLastCommittedURL();
+      DistillerPageWebContents::CreateNewWebContents(url);
+    }
+  }
+
+  virtual ~TestDistillerPageWebContents() {
+    if (!use_new_web_contents_) {
+      // Intentionally leaking WebContents, since it is owned by the shell.
+      content::WebContents* web_contents = web_contents_.release();
+      web_contents->GetLastCommittedURL();
+    }
+  }
+
+  bool new_web_contents_created() { return new_web_contents_created_; }
+
+ private:
+  bool use_new_web_contents_;
+  bool new_web_contents_created_;
+};
+
+// Helper class to know how far in the loading process the current WebContents
+// has come. It will call the callback either after
+// DidCommitProvisionalLoadForFrame or DocumentLoadedInFrame is called for the
+// main frame, based on the value of |wait_for_document_loaded|.
+class WebContentsMainFrameHelper : public content::WebContentsObserver {
+ public:
+  WebContentsMainFrameHelper(content::WebContents* web_contents,
+                             const base::Closure& callback,
+                             bool wait_for_document_loaded)
+      : web_contents_(web_contents),
+        callback_(callback),
+        wait_for_document_loaded_(wait_for_document_loaded) {
+    content::WebContentsObserver::Observe(web_contents);
+  }
+
+  virtual void DidCommitProvisionalLoadForFrame(
+      int64 frame_id,
+      const base::string16& frame_unique_name,
+      bool is_main_frame,
+      const GURL& url,
+      content::PageTransition transition_type,
+      content::RenderViewHost* render_view_host) OVERRIDE {
+    if (wait_for_document_loaded_)
+      return;
+    if (is_main_frame)
+      callback_.Run();
+  }
+
+  virtual void DocumentLoadedInFrame(
+      int64 frame_id,
+      content::RenderViewHost* render_view_host) OVERRIDE {
+    if (wait_for_document_loaded_) {
+      if (web_contents_ &&
+          frame_id == web_contents_->GetMainFrame()->GetRoutingID()) {
+        callback_.Run();
+      }
+    }
+  }
+
+ private:
+  content::WebContents* web_contents_;
+  base::Closure callback_;
+  bool wait_for_document_loaded_;
+};
+
 IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, BasicDistillationWorks) {
   DistillerPageWebContents distiller_page(
-      shell()->web_contents()->GetBrowserContext());
+      shell()->web_contents()->GetBrowserContext(),
+      scoped_ptr<SourcePageHandleWebContents>());
   distiller_page_ = &distiller_page;
 
   base::RunLoop run_loop;
-  DistillPage(run_loop.QuitClosure(), "/simple_article.html");
+  DistillPage(run_loop.QuitClosure(), kSimpleArticlePath);
   run_loop.Run();
 
   EXPECT_EQ("Test Page Title", page_info_.get()->title);
@@ -88,11 +186,12 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, BasicDistillationWorks) {
 
 IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeLinks) {
   DistillerPageWebContents distiller_page(
-      shell()->web_contents()->GetBrowserContext());
+      shell()->web_contents()->GetBrowserContext(),
+      scoped_ptr<SourcePageHandleWebContents>());
   distiller_page_ = &distiller_page;
 
   base::RunLoop run_loop;
-  DistillPage(run_loop.QuitClosure(), "/simple_article.html");
+  DistillPage(run_loop.QuitClosure(), kSimpleArticlePath);
   run_loop.Run();
 
   // A relative link should've been updated.
@@ -104,11 +203,12 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeLinks) {
 
 IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeImages) {
   DistillerPageWebContents distiller_page(
-      shell()->web_contents()->GetBrowserContext());
+      shell()->web_contents()->GetBrowserContext(),
+      scoped_ptr<SourcePageHandleWebContents>());
   distiller_page_ = &distiller_page;
 
   base::RunLoop run_loop;
-  DistillPage(run_loop.QuitClosure(), "/simple_article.html");
+  DistillPage(run_loop.QuitClosure(), kSimpleArticlePath);
   run_loop.Run();
 
   // A relative link should've been updated.
@@ -120,7 +220,8 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, HandlesRelativeImages) {
 
 IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, VisibilityDetection) {
   DistillerPageWebContents distiller_page(
-      shell()->web_contents()->GetBrowserContext());
+      shell()->web_contents()->GetBrowserContext(),
+      scoped_ptr<SourcePageHandleWebContents>());
   distiller_page_ = &distiller_page;
 
   // visble_style.html and invisible_style.html only differ by the visibility
@@ -141,4 +242,93 @@ IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest, VisibilityDetection) {
   }
 }
 
+IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest,
+                       UsingCurrentWebContentsWrongUrl) {
+  std::string url("about:blank");
+  bool use_new_web_contents = true;
+  bool setup_main_frame_observer = true;
+  bool wait_for_document_loaded = true;
+  RunUseCurrentWebContentsTest(url,
+                               use_new_web_contents,
+                               setup_main_frame_observer,
+                               wait_for_document_loaded);
+}
+
+IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest,
+                       UsingCurrentWebContentsNoMainFrameObserver) {
+  std::string url(kSimpleArticlePath);
+  bool use_new_web_contents = true;
+  bool setup_main_frame_observer = false;
+  bool wait_for_document_loaded = true;
+  RunUseCurrentWebContentsTest(url,
+                               use_new_web_contents,
+                               setup_main_frame_observer,
+                               wait_for_document_loaded);
+}
+
+IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest,
+                       UsingCurrentWebContentsNotFinishedLoadingYet) {
+  std::string url(kSimpleArticlePath);
+  bool use_new_web_contents = false;
+  bool setup_main_frame_observer = true;
+  bool wait_for_document_loaded = false;
+  RunUseCurrentWebContentsTest(url,
+                               use_new_web_contents,
+                               setup_main_frame_observer,
+                               wait_for_document_loaded);
+}
+
+IN_PROC_BROWSER_TEST_F(DistillerPageWebContentsTest,
+                       UsingCurrentWebContentsReadyForDistillation) {
+  std::string url(kSimpleArticlePath);
+  bool use_new_web_contents = false;
+  bool setup_main_frame_observer = true;
+  bool wait_for_document_loaded = true;
+  RunUseCurrentWebContentsTest(url,
+                               use_new_web_contents,
+                               setup_main_frame_observer,
+                               wait_for_document_loaded);
+}
+
+void DistillerPageWebContentsTest::RunUseCurrentWebContentsTest(
+    const std::string& url,
+    bool use_new_web_contents,

[Yaron, 2014/05/22 18:25:02]

nit: expect_new_web_contents throughout as this is expectation not how to run.

[nyquist, 2014/05/22 23:00:54]

Done.

+    bool setup_main_frame_observer,
+    bool wait_for_document_loaded) {
+  content::WebContents* current_web_contents = shell()->web_contents();
+  if (setup_main_frame_observer) {
+    dom_distiller::WebContentsMainFrameObserver::CreateForWebContents(
+        current_web_contents);
+  }
+  base::RunLoop url_loaded_runner;
+  scoped_ptr<WebContentsMainFrameHelper> main_frame_loaded(

[Yaron, 2014/05/22 18:25:02]

why scoped_ptr?

[nyquist, 2014/05/22 23:00:54]

Done.

+      new WebContentsMainFrameHelper(current_web_contents,
+                                     url_loaded_runner.QuitClosure(),
+                                     wait_for_document_loaded));
+  current_web_contents->GetController().LoadURL(
+      embedded_test_server()->GetURL(url),
+      content::Referrer(),
+      content::PAGE_TRANSITION_TYPED,
+      std::string());
+  url_loaded_runner.Run();
+
+  scoped_ptr<content::WebContents> old_web_contents_sptr(current_web_contents);
+  scoped_ptr<SourcePageHandleWebContents> source_page_handle(
+      new SourcePageHandleWebContents(old_web_contents_sptr.Pass()));
+
+  TestDistillerPageWebContents distiller_page(
+      shell()->web_contents()->GetBrowserContext(),
+      source_page_handle.Pass(),
+      use_new_web_contents);
+  distiller_page_ = &distiller_page;
+
+  base::RunLoop run_loop;
+  DistillPage(run_loop.QuitClosure(), kSimpleArticlePath);
+  run_loop.Run();
+
+  // Sanity check of distillation process.
+  EXPECT_EQ(use_new_web_contents, distiller_page.new_web_contents_created());
+  EXPECT_EQ("Test Page Title", page_info_.get()->title);
+}
+
 }  // namespace dom_distiller