Add support for displaying distilled articles.

Adds support in the content::URLDataSource for DOM Distiller for displaying
distilled articles.

Also adds preliminary error handling.

Depends on https://codereview.chromium.org/105723002/

BUG=319881

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247645 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/dom_distiller/dom_distiller_service_factory.cc

@@ -44,12 +44,9 @@ DomDistillerServiceFactory::~DomDistillerServiceFactory() {}
 
 BrowserContextKeyedService* DomDistillerServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* profile) const {
-  content::URLDataSource::Add(
-      profile, new DomDistillerViewerSource(chrome::kDomDistillerScheme));
-
   scoped_refptr<base::SequencedTaskRunner> background_task_runner =
-          content::BrowserThread::GetBlockingPool()->GetSequencedTaskRunner(
-              content::BrowserThread::GetBlockingPool()->GetSequenceToken());
+      content::BrowserThread::GetBlockingPool()->GetSequencedTaskRunner(
+          content::BrowserThread::GetBlockingPool()->GetSequenceToken());
 
   scoped_ptr<DomDistillerDatabase> db(
       new DomDistillerDatabase(background_task_runner));
@@ -57,21 +54,27 @@ BrowserContextKeyedService* DomDistillerServiceFactory::BuildServiceInstanceFor(
   base::FilePath database_dir(
       profile->GetPath().Append(FILE_PATH_LITERAL("Articles")));
 
-  scoped_ptr<DomDistillerStore> dom_distiller_store(
-      new DomDistillerStore(db.PassAs<DomDistillerDatabaseInterface>(),
-                            database_dir));
+  scoped_ptr<DomDistillerStore> dom_distiller_store(new DomDistillerStore(
+      db.PassAs<DomDistillerDatabaseInterface>(), database_dir));
 
   scoped_ptr<DistillerPageFactory> distiller_page_factory(
       new DistillerPageWebContentsFactory(profile));
   scoped_ptr<DistillerURLFetcherFactory> distiller_url_fetcher_factory(
       new DistillerURLFetcherFactory(profile->GetRequestContext()));
-  scoped_ptr<DistillerFactory> distiller_factory(
-      new DistillerFactoryImpl(distiller_page_factory.Pass(),
-                               distiller_url_fetcher_factory.Pass()));
-  return new DomDistillerContextKeyedService(
-      dom_distiller_store.PassAs<DomDistillerStoreInterface>(),
-            distiller_factory.Pass());
+  scoped_ptr<DistillerFactory> distiller_factory(new DistillerFactoryImpl(
+      distiller_page_factory.Pass(), distiller_url_fetcher_factory.Pass()));
+
+  DomDistillerContextKeyedService* service =
+      new DomDistillerContextKeyedService(
+          dom_distiller_store.PassAs<DomDistillerStoreInterface>(),
+          distiller_factory.Pass());
+
+  // Set up URL data source for the chrome-distiller:// scheme.
+  content::URLDataSource::Add(
+      profile,
+      new DomDistillerViewerSource(service, chrome::kDomDistillerScheme));
 
+  return service;
 }
 
 content::BrowserContext* DomDistillerServiceFactory::GetBrowserContextToUse(

chrome/browser/dom_distiller/dom_distiller_viewer_source_browsertest.cc

@@ -5,6 +5,7 @@
 #include <string.h>
 
 #include "base/command_line.h"
+#include "chrome/browser/dom_distiller/dom_distiller_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -13,6 +14,13 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/dom_distiller/content/dom_distiller_viewer_source.h"
+#include "components/dom_distiller/core/distiller.h"
+#include "components/dom_distiller/core/dom_distiller_service.h"
+#include "components/dom_distiller/core/dom_distiller_store.h"
+#include "components/dom_distiller/core/dom_distiller_test_util.h"
+#include "components/dom_distiller/core/fake_db.h"
+#include "components/dom_distiller/core/fake_distiller.h"
+#include "components/dom_distiller/core/task_tracker.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/url_data_source.h"
 #include "content/public/browser/web_contents.h"
@@ -21,6 +29,29 @@
 
 namespace dom_distiller {
 
+using test::FakeDB;
+using test::FakeDistiller;
+using test::MockDistillerFactory;
+using test::util::CreateStoreWithFakeDB;
+
+namespace {
+
+void AddEntry(const ArticleEntry& e, FakeDB::EntryMap* map) {
+  (*map)[e.entry_id()] = e;
+}
+
+ArticleEntry CreateEntry(std::string entry_id, std::string page_url) {
+  ArticleEntry entry;
+  entry.set_entry_id(entry_id);
+  if (!page_url.empty()) {
+    ArticleEntryPage* page = entry.add_pages();
+    page->set_url(page_url);
+  }
+  return entry;
+}
+
+}  // namespace
+
 // WebContents observer that stores reference to the current |RenderViewHost|.
 class LoadSuccessObserver : public content::WebContentsObserver {
  public:
@@ -29,6 +60,7 @@ class LoadSuccessObserver : public content::WebContentsObserver {
         validated_url_(GURL()),
         finished_load_(false),
         load_failed_(false),
+        web_contents_(contents),
         render_view_host_(NULL) {}
 
   virtual void DidFinishLoad(int64 frame_id,
@@ -55,6 +87,7 @@ class LoadSuccessObserver : public content::WebContentsObserver {
   const GURL& validated_url() const { return validated_url_; }
   bool finished_load() const { return finished_load_; }
   bool load_failed() const { return load_failed_; }
+  content::WebContents* web_contents() const { return web_contents_; }
 
   const content::RenderViewHost* render_view_host() const {
     return render_view_host_;
@@ -64,6 +97,7 @@ class LoadSuccessObserver : public content::WebContentsObserver {
   GURL validated_url_;
   bool finished_load_;
   bool load_failed_;
+  content::WebContents* web_contents_;
   content::RenderViewHost* render_view_host_;
 
   DISALLOW_COPY_AND_ASSIGN(LoadSuccessObserver);
@@ -74,19 +108,78 @@ class DomDistillerViewerSourceBrowserTest : public InProcessBrowserTest {
   DomDistillerViewerSourceBrowserTest() {}
   virtual ~DomDistillerViewerSourceBrowserTest() {}
 
+  virtual void SetUpOnMainThread() OVERRIDE {
+    database_model_ = new FakeDB::EntryMap;
+  }
+
+  virtual void CleanUpOnMainThread() OVERRIDE { delete database_model_; }
+
   virtual void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     command_line->AppendSwitch(switches::kEnableDomDistiller);
   }
+
+  static BrowserContextKeyedService* Build(content::BrowserContext* context) {
+    FakeDB* fake_db = new FakeDB(database_model_);
+    MockDistillerFactory* factory = new MockDistillerFactory();
+    DomDistillerContextKeyedService* service =
+        new DomDistillerContextKeyedService(
+            scoped_ptr<DomDistillerStoreInterface>(
+                CreateStoreWithFakeDB(fake_db, FakeDB::EntryMap())),
+            scoped_ptr<DistillerFactory>(factory));
+    fake_db->InitCallback(true);
+    fake_db->LoadCallback(true);
+    if (expect_distillation_) {
+      // There will only be destillation of an article if the database contains
+      // the article.
+      FakeDistiller* distiller = new FakeDistiller(true);
+      EXPECT_CALL(*factory, CreateDistillerImpl())
+          .WillOnce(testing::Return(distiller));
+    }
+    return service;
+  }
+
+  void ViewSingleDistilledPage();
+
+  // Database entries.
+  static FakeDB::EntryMap* database_model_;
+  static bool expect_distillation_;
 };
 
+FakeDB::EntryMap* DomDistillerViewerSourceBrowserTest::database_model_;
+bool DomDistillerViewerSourceBrowserTest::expect_distillation_ = false;
+
 // The DomDistillerViewerSource renders untrusted content, so ensure no bindings
-// are enabled.
-IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, NoWebUIBindings) {
+// are enabled when the article exists in the database.
+IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest,
+                       NoWebUIBindingsArticleExists) {
+  // Ensure there is one item in the database, which will trigger distillation.
+  ArticleEntry entry = CreateEntry("DISTILLED", "http://example.com/1");
+  AddEntry(entry, database_model_);
+  expect_distillation_ = true;
+  ViewSingleDistilledPage();
+}
+
+// The DomDistillerViewerSource renders untrusted content, so ensure no bindings
+// are enabled when the article is not found.
+IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest,
+                       NoWebUIBindingsArticleNotFound) {
+  // The article does not exist, so assume no distillation will happen.
+  expect_distillation_ = false;
+  ViewSingleDistilledPage();
+}
+
+void DomDistillerViewerSourceBrowserTest::ViewSingleDistilledPage() {
+  // Create the service.
+  DomDistillerContextKeyedService* service =
+      static_cast<DomDistillerContextKeyedService*>(
+          dom_distiller::DomDistillerServiceFactory::GetInstance()
+              ->SetTestingFactoryAndUse(browser()->profile(), &Build));
+
   // Ensure the source is registered.
   // TODO(nyquist): Remove when the source is always registered on startup.
-  content::URLDataSource::Add(
-      browser()->profile(),
-      new DomDistillerViewerSource(chrome::kDomDistillerScheme));
+  DomDistillerViewerSource* source =
+      new DomDistillerViewerSource(service, chrome::kDomDistillerScheme);
+  content::URLDataSource::Add(browser()->profile(), source);
 
   // Setup observer to inspect the RenderViewHost after committed navigation.
   content::WebContents* contents =
@@ -105,6 +198,8 @@ IN_PROC_BROWSER_TEST_F(DomDistillerViewerSourceBrowserTest, NoWebUIBindings) {
   // Ensure no bindings.
   const content::RenderViewHost* render_view_host = observer.render_view_host();
   ASSERT_EQ(0, render_view_host->GetEnabledBindings());
+  // The MIME-type should always be text/html.
+  EXPECT_EQ("text/html", observer.web_contents()->GetContentsMimeType());
 }
 
 }  // namespace dom_distiller

components/dom_distiller.gypi

@@ -128,7 +128,10 @@
               'target_name': 'dom_distiller_content',
               'type': 'static_library',
               'dependencies': [
+                'component_strings.gyp:component_strings',
                 'dom_distiller_core',
+                'dom_distiller_resources',
+                '../net/net.gyp:net',
                 '../skia/skia.gyp:skia',
                 '../sync/sync.gyp:sync',
               ],

components/dom_distiller/content/DEPS

@@ -3,5 +3,7 @@ include_rules = [
   "+content/public",
   "+content/shell",
   "+content/test",
+  "+net/base",
   "+net/test",
+  "+ui/base/l10n",
 ]

components/dom_distiller/content/dom_distiller_viewer_source.cc

@@ -5,18 +5,97 @@
 #include "components/dom_distiller/content/dom_distiller_viewer_source.h"
 
 #include <string>
+#include <vector>
 
 #include "base/memory/ref_counted_memory.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "base/strings/string_util.h"
+#include "components/dom_distiller/core/dom_distiller_service.h"
+#include "components/dom_distiller/core/proto/distilled_page.pb.h"
+#include "components/dom_distiller/core/task_tracker.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
+#include "grit/component_strings.h"
+#include "grit/dom_distiller_resources.h"
+#include "net/base/escape.h"
 #include "net/url_request/url_request.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
 #include "url/gurl.h"
 
+namespace {
+
+std::string ReplaceHtmlTemplateValues(std::string title, std::string content) {
+  base::StringPiece html_template =
+      ResourceBundle::GetSharedInstance().GetRawDataResource(
+          IDR_DOM_DISTILLER_VIEWER_HTML);
+  std::vector<std::string> substitutions;
+  substitutions.push_back(title);    // $1
+  substitutions.push_back(title);    // $2
+  substitutions.push_back(content);  // $3
+  return ReplaceStringPlaceholders(html_template, substitutions, NULL);
+}
+
+}  // namespace
+
 namespace dom_distiller {
 
-DomDistillerViewerSource::DomDistillerViewerSource(const std::string& scheme)
-    : scheme_(scheme) {}
+// Handles receiving data asynchronously for a specific entry, and passing
+// it along to the data callback for the data source.
+class RequestViewerHandle : public ViewRequestDelegate {
+ public:
+  explicit RequestViewerHandle(
+      const content::URLDataSource::GotDataCallback& callback);
+  virtual ~RequestViewerHandle();
+
+  // ViewRequestDelegate implementation.
+  virtual void OnArticleReady(DistilledPageProto* proto) OVERRIDE;
+
+  void TakeViewerHandle(scoped_ptr<ViewerHandle> viewer_handle);
+
+ private:
+  // The handle to the view request towards the DomDistillerService. It
+  // needs to be kept around to ensure the distillation request finishes.
+  scoped_ptr<ViewerHandle> viewer_handle_;
+
+  // This holds the callback to where the data retrieved is sent back.
+  content::URLDataSource::GotDataCallback callback_;
+};
+
+RequestViewerHandle::RequestViewerHandle(
+    const content::URLDataSource::GotDataCallback& callback)
+    : callback_(callback) {}
+
+RequestViewerHandle::~RequestViewerHandle() {}
+
+void RequestViewerHandle::OnArticleReady(DistilledPageProto* proto) {
+  DCHECK(proto);
+  std::string title;
+  std::string unsafe_article_html;
+  if (proto->has_title() && proto->has_html()) {
+    title = net::EscapeForHTML(proto->title());
+    unsafe_article_html = proto->html();
+  } else {
+    title = l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_TITLE);
+    unsafe_article_html =
+        l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_NO_DATA_CONTENT);
+  }
+  std::string unsafe_page_html =
+      ReplaceHtmlTemplateValues(title, unsafe_article_html);
+  callback_.Run(base::RefCountedString::TakeString(&unsafe_page_html));
+  base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
+}
+
+void RequestViewerHandle::TakeViewerHandle(
+    scoped_ptr<ViewerHandle> viewer_handle) {
+  viewer_handle_ = viewer_handle.Pass();
+}
+
+DomDistillerViewerSource::DomDistillerViewerSource(
+    DomDistillerService* dom_distiller_service,
+    const std::string& scheme)
+    : scheme_(scheme), dom_distiller_service_(dom_distiller_service) {}
 
 DomDistillerViewerSource::~DomDistillerViewerSource() {}
 
@@ -35,13 +114,34 @@ void DomDistillerViewerSource::StartDataRequest(
   DCHECK(render_view_host);
   CHECK_EQ(0, render_view_host->GetEnabledBindings());
 
-  std::string page_template = "Aloha!";
-  callback.Run(base::RefCountedString::TakeString(&page_template));
+  RequestViewerHandle* request_viewer_handle =
+      new RequestViewerHandle(callback);
+  std::string entry_id = StringToUpperASCII(path);
+  scoped_ptr<ViewerHandle> viewer_handle =
+      dom_distiller_service_->ViewEntry(request_viewer_handle, entry_id);
+  if (viewer_handle) {
+    // The service returned a |ViewerHandle| and guarantees it will call
+    // the |RequestViewerHandle|, so passing ownership to it, to ensure the
+    // request is not cancelled. The |RequestViewerHandle| will delete itself
+    // after receiving the callback.
+    request_viewer_handle->TakeViewerHandle(viewer_handle.Pass());
+  } else {
+    // The service did not return a |ViewerHandle|, which means the
+    // |RequestViewerHandle| will never be called, so clean up now.
+    delete request_viewer_handle;
+
+    std::string title = l10n_util::GetStringUTF8(
+        IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_TITLE);
+    std::string content = l10n_util::GetStringUTF8(
+        IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_CONTENT);
+    std::string html = ReplaceHtmlTemplateValues(title, content);
+    callback.Run(base::RefCountedString::TakeString(&html));
+  }
 };
 
 std::string DomDistillerViewerSource::GetMimeType(const std::string& path)
     const {
-  return "text/plain";
+  return "text/html";
 }
 
 bool DomDistillerViewerSource::ShouldServiceRequest(
@@ -49,4 +149,12 @@ bool DomDistillerViewerSource::ShouldServiceRequest(
   return request->url().SchemeIs(scheme_.c_str());
 }
 
+void DomDistillerViewerSource::WillServiceRequest(
+    const net::URLRequest* request,
+    std::string* path) const {
+  // Since the full request is not available to StartDataRequest, replace the
+  // path to contain the data needed.
+  *path = request->url().host();
+};
+
 }  // namespace dom_distiller

components/dom_distiller/content/dom_distiller_viewer_source.h

@@ -11,14 +11,16 @@
 
 namespace dom_distiller {
 
+class DomDistillerService;
+
 // Serves HTML and resources for viewing distilled articles.
 class DomDistillerViewerSource : public content::URLDataSource {
  public:
-  DomDistillerViewerSource(const std::string& scheme);
-
- private:
+  DomDistillerViewerSource(DomDistillerService* dom_distiller_service,
+                           const std::string& scheme);
   virtual ~DomDistillerViewerSource();
 
+ private:
   // Overridden from content::URLDataSource:
   virtual std::string GetSource() const OVERRIDE;
   virtual void StartDataRequest(
@@ -27,12 +29,18 @@ class DomDistillerViewerSource : public content::URLDataSource {
       int render_frame_id,
       const content::URLDataSource::GotDataCallback& callback) OVERRIDE;
   virtual std::string GetMimeType(const std::string& path) const OVERRIDE;
-  virtual bool ShouldServiceRequest(
-      const net::URLRequest* request) const OVERRIDE;
+  virtual bool ShouldServiceRequest(const net::URLRequest* request)
+      const OVERRIDE;
+  virtual void WillServiceRequest(const net::URLRequest* request,
+                                  std::string* path) const OVERRIDE;
 
   // The scheme this URLDataSource is hosted under.
   std::string scheme_;
 
+  // The service which contains all the functionality needed to interact with
+  // the list of articles.
+  DomDistillerService* dom_distiller_service_;
+
   DISALLOW_COPY_AND_ASSIGN(DomDistillerViewerSource);
 };
 

components/dom_distiller/content/resources/dom_distiller_viewer.html

+<!DOCTYPE HTML>
+<!--
+Copyright 2014 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.
+-->
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>$1</title>
+</head>
+<body>
+  <div id="mainContent">
+    <div id="article">
+      <article>
+        <header>
+          <h1>$2</h1>
+        </header>
+        <div id="content">$3</div>
+      </article>
+    </div>
+  </div>
+</body>
+</html>

components/dom_distiller_resources.grd

@@ -12,6 +12,7 @@
       <include name="IDR_ABOUT_DOM_DISTILLER_HTML" file="dom_distiller/webui/resources/about_dom_distiller.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" />
       <include name="IDR_ABOUT_DOM_DISTILLER_CSS" file="dom_distiller/webui/resources/about_dom_distiller.css" type="BINDATA" />
       <include name="IDR_ABOUT_DOM_DISTILLER_JS" file="dom_distiller/webui/resources/about_dom_distiller.js" type="BINDATA" />
+      <include name="IDR_DOM_DISTILLER_VIEWER_HTML" file="dom_distiller/content/resources/dom_distiller_viewer.html" type="BINDATA" />
       <include name="IDR_DISTILLER_JS" file="../third_party/readability/js/readability.js" type="BINDATA" />
     </includes>
   </release>

components/dom_distiller_strings.grdp

@@ -22,5 +22,17 @@
   <message name="IDS_DOM_DISTILLER_WEBUI_FETCHING_ENTRIES" desc="The text to show while fetching the list of entries on the DOM Distiller debug page.">
     Fetching entries...
   </message>
+  <message name="IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_TITLE" desc="The text to show in place of a reading list article title if the article is not found.">
+    Failed to find article.
+  </message>
+  <message name="IDS_DOM_DISTILLER_VIEWER_FAILED_TO_FIND_ARTICLE_CONTENT" desc="The text to show in place of reading list article content if the article is not found.">
+    Could not find the requested article.
+  </message>
+  <message name="IDS_DOM_DISTILLER_VIEWER_NO_DATA_TITLE" desc="The text to show in place of a reading list article title if there is no data found.">
+    Failed to display article.
+  </message>
+  <message name="IDS_DOM_DISTILLER_VIEWER_NO_DATA_CONTENT" desc="The text to show in place of reading list article content if there is no data found.">
+    No data found.
+  </message>
 
 </grit-part>