Add support for distilling arbitrary URLs in DOM Distiller Viewer.

components/dom_distiller/webui/dom_distiller_handler.cc

 // Copyright 2013 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/webui/dom_distiller_handler.h"
 
 #include <vector>
 
 #include "base/bind.h"
+#include "base/guid.h"

[shashi, 2014/02/27 04:29:08]

nit: is this import needed?

[shashi, 2014/02/27 04:29:08]

nit: is this import needed?

[nyquist, 2014/02/27 19:50:19]

Done.

 #include "base/values.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/url_constants.h"

[shashi, 2014/02/27 04:29:08]

nit: is this import needed?

[shashi, 2014/02/27 04:29:08]

nit: is this import needed?

[nyquist, 2014/02/27 19:50:19]

Done.

+#include "components/dom_distiller/core/url_utils.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
+#include "net/base/escape.h"
 #include "url/gurl.h"
 
 namespace dom_distiller {
 
 DomDistillerHandler::DomDistillerHandler(DomDistillerService* service,
                                          const std::string& scheme)
     : service_(service), article_scheme_(scheme), weak_ptr_factory_(this) {}
 
 DomDistillerHandler::~DomDistillerHandler() {}
 
 void DomDistillerHandler::RegisterMessages() {
   web_ui()->RegisterMessageCallback(
       "requestEntries",
       base::Bind(&DomDistillerHandler::HandleRequestEntries,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "addArticle",
       base::Bind(&DomDistillerHandler::HandleAddArticle,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback(
       "selectArticle",
       base::Bind(&DomDistillerHandler::HandleSelectArticle,
                  base::Unretained(this)));
+  web_ui()->RegisterMessageCallback(
+      "viewUrl",
+      base::Bind(&DomDistillerHandler::HandleViewUrl, base::Unretained(this)));
 }
 
 void DomDistillerHandler::HandleAddArticle(const base::ListValue* args) {
   std::string url;
   args->GetString(0, &url);
   GURL gurl(url);
   if (gurl.is_valid()) {
     service_->AddToList(
         gurl,
         base::Bind(base::Bind(&DomDistillerHandler::OnArticleAdded,
                               base::Unretained(this))));
   } else {
     web_ui()->CallJavascriptFunction("domDistiller.onArticleAddFailed");
   }
 }
 
+void DomDistillerHandler::HandleViewUrl(const base::ListValue* args) {
+  std::string url;
+  args->GetString(0, &url);
+  const GURL gurl(url);
+  if (gurl.is_valid()) {
+    GURL view_url =
+        url_utils::GetDistillerViewUrlFromUrl(article_scheme_, gurl);
+    DCHECK(view_url.is_valid());

[shashi, 2014/02/27 04:29:08]

nit: should it be DCHECK(url_utils::IsDistillableUrl(view_url))?

[nyquist, 2014/02/27 19:50:19]

It should be that on the check of gurl above. This URL is just the one we
create. Thanks!

+    web_ui()->GetWebContents()->GetController().LoadURL(
+        view_url,
+        content::Referrer(),
+        content::PAGE_TRANSITION_GENERATED,
+        std::string());
+  } else {
+    web_ui()->CallJavascriptFunction("domDistiller.onViewUrlFailed");
+  }
+}
+
 void DomDistillerHandler::HandleSelectArticle(const base::ListValue* args) {
   std::string entry_id;
   args->GetString(0, &entry_id);
-  GURL url(article_scheme_ + std::string("://") + entry_id);
+  GURL url =
+      url_utils::GetDistillerViewUrlFromEntryId(article_scheme_, entry_id);
   DCHECK(url.is_valid());
-  web_ui()->GetWebContents()->GetController().LoadURL(url,
-      content::Referrer(), content::PAGE_TRANSITION_GENERATED,
+  web_ui()->GetWebContents()->GetController().LoadURL(
+      url,
+      content::Referrer(),
+      content::PAGE_TRANSITION_GENERATED,
       std::string());
 }
 
 void DomDistillerHandler::HandleRequestEntries(const base::ListValue* args) {
   base::ListValue entries;
   const std::vector<ArticleEntry>& entries_specifics = service_->GetEntries();
   for (std::vector<ArticleEntry>::const_iterator it = entries_specifics.begin();
        it != entries_specifics.end();
        ++it) {
     const ArticleEntry& article = *it;
     DCHECK(IsEntryValid(article));
     scoped_ptr<base::DictionaryValue> entry(new base::DictionaryValue());
     entry->SetString("entry_id", article.entry_id());
     std::string title = (!article.has_title() || article.title().empty())
                             ? article.entry_id()
                             : article.title();
-    entry->SetString("title", title);
+    entry->SetString("title", net::EscapeForHTML(title));

[shashi, 2014/02/27 04:29:08]

Is there a unit test somewhere to test all values that we display under
chrome-distiller are actually escaped?. Like initialize a mock service with
article : { 'title': "<javascript> alert('foo'); </javascript>", 'html' : }

Note: you do not need to add this test in the current CL.

[nyquist, 2014/02/27 19:50:19]

Good suggestion. I'll add a TODO for that.

     entries.Append(entry.release());
   }
   web_ui()->CallJavascriptFunction("domDistiller.onReceivedEntries", entries);
 }
 
 void DomDistillerHandler::OnArticleAdded(bool article_available) {
   // TODO(nyquist): Update this function.
   if (article_available) {
     HandleRequestEntries(NULL);
   } else {
     web_ui()->CallJavascriptFunction("domDistiller.onArticleAddFailed");
   }
 }
 
 }  // namespace dom_distiller