Add support for distilling arbitrary URLs in DOM Distiller Viewer.

components/dom_distiller/content/dom_distiller_viewer_source.cc

 // 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.
 
 #include "components/dom_distiller/content/dom_distiller_viewer_source.h"
 
 #include <sstream>
 #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_article.pb.h"
 #include "components/dom_distiller/core/proto/distilled_page.pb.h"
 #include "components/dom_distiller/core/task_tracker.h"
+#include "components/dom_distiller/core/url_constants.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/base/url_util.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 {
 
 const char kCssPath[] = "readability.css";
+const char kRequestIdKey[] = "request_id";
 
 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(kCssPath);  // $2
-  substitutions.push_back(title);     // $3
-  substitutions.push_back(content);   // $4
+  substitutions.push_back(title);                        // $1
+  substitutions.push_back(std::string("/") + kCssPath);  // $2

[cjhopman, 2014/02/05 23:03:06]

why not put this slash in the html file?

[nyquist, 2014/02/25 20:30:24]

Done.

+  substitutions.push_back(title);                        // $3
+  substitutions.push_back(content);                      // $4
   return ReplaceStringPlaceholders(html_template, substitutions, NULL);
 }
 
 }  // namespace
 
 namespace dom_distiller {
 
 // Handles receiving data asynchronously for a specific entry, and passing
 // it along to the data callback for the data source.
-class RequestViewerHandle : public ViewRequestDelegate {
+class DomDistillerViewerSource::RequestViewerHandle
+    : public ViewRequestDelegate {
  public:
   explicit RequestViewerHandle(
       const content::URLDataSource::GotDataCallback& callback);
   virtual ~RequestViewerHandle();
 
   // ViewRequestDelegate implementation.
   virtual void OnArticleReady(const DistilledArticleProto* article_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(
+DomDistillerViewerSource::RequestViewerHandle::RequestViewerHandle(
     const content::URLDataSource::GotDataCallback& callback)
     : callback_(callback) {}
 
-RequestViewerHandle::~RequestViewerHandle() {}
+DomDistillerViewerSource::RequestViewerHandle::~RequestViewerHandle() {}
 
-void RequestViewerHandle::OnArticleReady(
+void DomDistillerViewerSource::RequestViewerHandle::OnArticleReady(
     const DistilledArticleProto* article_proto) {
   DCHECK(article_proto);
   std::string title;
   std::string unsafe_article_html;
   if (article_proto->has_title() && article_proto->pages_size() > 0 &&
       article_proto->pages(0).has_html()) {
     title = net::EscapeForHTML(article_proto->title());
     // TODO(shashishekhar): Add support for correcting displaying multiple pages
     // after discussing the right way to display them.
     std::ostringstream unsafe_output_stream;
     for (int page_num = 0; page_num < article_proto->pages_size(); ++page_num) {
       unsafe_output_stream << article_proto->pages(page_num).html();
     }
     unsafe_article_html = unsafe_output_stream.str();
   } 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(
+void DomDistillerViewerSource::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() {}
@@ skipping 18 matching lines @@
   if (kCssPath == path) {
     std::string css = ResourceBundle::GetSharedInstance()
                           .GetRawDataResource(IDR_DISTILLER_CSS)
                           .as_string();
     callback.Run(base::RefCountedString::TakeString(&css));
     return;
   }
 
   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);
+      CreateViewRequest(path, request_viewer_handle);
+
   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;
@@ skipping 15 matching lines @@
 }
 
 bool DomDistillerViewerSource::ShouldServiceRequest(
     const net::URLRequest* request) const {
   return request->url().SchemeIs(scheme_.c_str());
 }
 
 void DomDistillerViewerSource::WillServiceRequest(
     const net::URLRequest* request,
     std::string* path) const {
-  if (*path != kCssPath) {
-    // Since the full request is not available to StartDataRequest, replace the
-    // path to contain the data needed.
-    *path = request->url().host();
-  }
+  if (*path == kCssPath)
+    return;
+  GURL dummy_url(scheme_ + "://dummy/?" + request->url().query());

[shashi, 2014/02/05 22:28:08]

nit: Instead of using dummy, maybe use dom-distiller-viewer-internal or some
other scheme, so that when I see this in the log, I can make sense of the url.

Also add a comment, that host is not passed when starting data request so we are
modifying the path to include the host component. This does feel a bit hacky
though.

[nyquist, 2014/02/25 20:30:24]

Done.

+  // Use the host component as the request ID.
+  GURL url = net::AppendOrReplaceQueryParameter(

[shashi, 2014/02/05 22:28:08]

Maybe a unit_test to make sure this works with urls that already have
request_id. also testing when request_id is actually in the distilled url: e.g. 
?url="http://example.com/request_id=abc"

[nyquist, 2014/02/25 20:30:24]

Removed request_id now.

+      dummy_url, kRequestIdKey, request->url().host());

[cjhopman, 2014/02/05 23:03:06]

It looks like the request id is never used.

[nyquist, 2014/02/25 20:30:24]

Done.

+  *path = url.query();
 };
 
 std::string DomDistillerViewerSource::GetContentSecurityPolicyObjectSrc()
     const {
-  return "object-src 'none'; style-src 'self'";
+  return "object-src 'none'; style-src 'self';";
+}
+
+scoped_ptr<ViewerHandle> DomDistillerViewerSource::CreateViewRequest(
+    const std::string& path,
+    RequestViewerHandle* request_viewer_handle) {
+  GURL dummy_url(scheme_ + "://dummy/?" + path);

[cjhopman, 2014/02/05 23:03:06]

I don't really like how we are passing data around here. I'd really like to see
us figure out the long-term plan for this view stuff.

[cjhopman, 2014/02/05 23:03:06]

Could you comment on what you need this dummy_url for? Or extract it to a nicely
named function?

Maybe just add a function like "GetValueForKeyInURLPathQuery" either here or in
url_utils that constructs the dummy_url itself.

[nyquist, 2014/02/25 20:30:24]

Done.

+  std::string request_id;
+  net::GetValueForKeyInQuery(dummy_url, kRequestIdKey, &request_id);
+  std::string entry_id;
+  net::GetValueForKeyInQuery(dummy_url, kEntryIdKey, &entry_id);
+  bool has_entry_id = !entry_id.empty();
+  entry_id = StringToUpperASCII(entry_id);
+
+  std::string requested_url_str;
+  net::GetValueForKeyInQuery(dummy_url, kUrlKey, &requested_url_str);
+  GURL requested_url(requested_url_str);
+  bool has_url = !requested_url_str.empty() && requested_url.is_valid();
+
+  if (has_entry_id) {
+    return dom_distiller_service_->ViewEntry(request_viewer_handle, entry_id)
+        .Pass();
+  } else if (has_url) {
+    return dom_distiller_service_->ViewUrl(request_viewer_handle, requested_url)
+        .Pass();
+  } else {
+    return scoped_ptr<ViewerHandle>();
+  }
 }
 
 }  // namespace dom_distiller