Add ContentSuggestionsService to SnippetsInternals

chrome/browser/ui/webui/snippets_internals_message_handler.cc

 // Copyright 2016 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 "chrome/browser/ui/webui/snippets_internals_message_handler.h"
 
 #include <memory>
 #include <set>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/i18n/time_formatting.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/values.h"
 #include "chrome/browser/android/chrome_feature_list.h"
+#include "chrome/browser/ntp_snippets/content_suggestions_service_factory.h"
 #include "chrome/browser/ntp_snippets/ntp_snippets_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/ntp_snippets/ntp_snippet.h"
 #include "components/ntp_snippets/switches.h"
 #include "content/public/browser/web_ui.h"
 
+using ntp_snippets::ContentSuggestion;
+using ntp_snippets::ContentSuggestionsCategory;
+using ntp_snippets::ContentSuggestionsCategoryStatus;
+
 namespace {
 
 std::unique_ptr<base::DictionaryValue> PrepareSnippet(
     const ntp_snippets::NTPSnippet& snippet,
     int index,
     bool discarded) {
   std::unique_ptr<base::DictionaryValue> entry(new base::DictionaryValue);
   entry->SetString("snippetId", snippet.id());
   entry->SetString("title", snippet.title());
   entry->SetString("siteTitle", snippet.best_source().publisher_name);
   entry->SetString("snippet", snippet.snippet());
   entry->SetString("published",
                    TimeFormatShortDateAndTime(snippet.publish_date()));
   entry->SetString("expires",
                    TimeFormatShortDateAndTime(snippet.expiry_date()));
   entry->SetString("url", snippet.best_source().url.spec());
   entry->SetString("ampUrl", snippet.best_source().amp_url.spec());
   entry->SetString("salientImageUrl", snippet.salient_image_url().spec());
   entry->SetDouble("score", snippet.score());
 
   if (discarded)
     entry->SetString("id", "discarded-snippet-" + base::IntToString(index));
   else
     entry->SetString("id", "snippet-" + base::IntToString(index));
 
   return entry;
 }
 
+std::unique_ptr<base::DictionaryValue> PrepareSuggestion(
+    const ntp_snippets::ContentSuggestion& suggestion,

[Marc Treib, 2016/07/12 13:04:52]

nit: ntp_snippets:: not needed, you have a "using" above

[Philipp Keck, 2016/07/12 13:45:02]

Done.

+    int index) {
+  std::unique_ptr<base::DictionaryValue> entry(new base::DictionaryValue);

[Bernhard Bauer, 2016/07/12 13:09:45]

You might be able to do `auto entry = base::MakeUnique<base::DictionaryValue>`?

[Philipp Keck, 2016/07/12 13:45:02]

Done.

+  entry->SetString("suggestionId", suggestion.id());
+  entry->SetString("url", suggestion.url().spec());
+  entry->SetString("ampUrl", suggestion.amp_url().spec());
+  entry->SetString("title", suggestion.title());
+  entry->SetString("snippetText", suggestion.snippet_text());
+  entry->SetString("publishDate",
+                   TimeFormatShortDateAndTime(suggestion.publish_date()));
+  entry->SetString("publisherName", suggestion.publisher_name());
+  entry->SetString("id", "content-suggestion-" + base::IntToString(index));
+  return entry;
+}
+
+std::string MapCategoryName(ContentSuggestionsCategory category) {
+  switch (category) {
+    case ContentSuggestionsCategory::ARTICLES:
+      return "Articles";
+    default:

[Bernhard Bauer, 2016/07/12 13:09:45]

If you remove the default case from the switch statement, the compiler will warn
(which is then turned into an error) if you forget a value, which might come in
handy when you add more categories.

[Philipp Keck, 2016/07/12 13:45:02]

Done. Oh great. I didn't know it would warn and that's exactly why I added the
default in the first place (even though it's currently dead code).

So I can remove it here, too, I assume:
https://codereview.chromium.org/2131943002/diff/200001/components/ntp_snippet...

And now there is a case for the "COUNT" dummy value...

[Bernhard Bauer, 2016/07/12 14:44:51]

Indeed!
Yup, that's one of the disadvantages, but reeallly, the COUNT value shouldn't be
part of the enum... :)

I think you'll have to move the old default handling code outside of the switch
now though (you can keep the section for COUNT empty), otherwise the compiler
will complain that you're missing a return statement.

[Philipp Keck, 2016/07/12 15:19:56]

Done. (other CL)
If the COUNT value wasn't there, how would you check whether a given "int" is a
valid value for the enum?

(Not sure if the empty COUNT section with the return statement after the switch
is more desirable than the current code -- but not having a COUNT section at all
would be nice.)

[Bernhard Bauer, 2016/07/12 15:50:31]

You could have an int (not an enum) that represents the maximum value (or the
maximum + 1, or something). I don't know of a way at the moment though to make
that work automatically, so my comment was more hypothetical. Sorry for not
making that clear!

[Philipp Keck, 2016/07/12 16:05:36]

You'd always have to remember to update that int, though, when you change the
contents of the enum. In my opinion, C++ should support sth like that out of the
box, at least with their "enum class" ...
The only solutions without compromises that I saw on the web use macros.

I'll leave the code here as is (not as seen here, but there:
https://codereview.chromium.org/2145563002/diff/40001/chrome/browser/ui/webui...)

[Bernhard Bauer, 2016/07/12 16:16:27]

Yes, precisely.

+      return "Unknown category " + base::IntToString(int(category));
+  }
+}
+
+std::string MapCategoryStatus(ContentSuggestionsCategoryStatus status) {
+  switch (status) {
+    case ContentSuggestionsCategoryStatus::INITIALIZING:
+      return "INITIALIZING";
+    case ContentSuggestionsCategoryStatus::AVAILABLE:
+      return "AVAILABLE";
+    case ContentSuggestionsCategoryStatus::AVAILABLE_LOADING:
+      return "AVAILABLE_LOADING";
+    case ContentSuggestionsCategoryStatus::NOT_PROVIDED:
+      return "NOT_PROVIDED";
+    case ContentSuggestionsCategoryStatus::ALL_SUGGESTIONS_EXPLICITLY_DISABLED:
+      return "ALL_SUGGESTIONS_EXPLICITLY_DISABLED";
+    case ContentSuggestionsCategoryStatus::CATEGORY_EXPLICITLY_DISABLED:
+      return "CATEGORY_EXPLICITLY_DISABLED";
+    case ContentSuggestionsCategoryStatus::SIGNED_OUT:
+      return "SIGNED_OUT";
+    case ContentSuggestionsCategoryStatus::SYNC_DISABLED:
+      return "SYNC_DISABLED";
+    case ContentSuggestionsCategoryStatus::PASSPHRASE_ENCRYPTION_ENABLED:
+      return "PASSPHRASE_ENCRYPTION_ENABLED";
+    case ContentSuggestionsCategoryStatus::HISTORY_SYNC_DISABLED:
+      return "HISTORY_SYNC_DISABLED";
+    case ContentSuggestionsCategoryStatus::LOADING_ERROR:
+      return "LOADING_ERROR";
+    default:

[Bernhard Bauer, 2016/07/12 13:09:45]

Same here.

[Philipp Keck, 2016/07/12 13:45:02]

Done.

+      return "Unknown status: " + base::IntToString(int(status));

[Bernhard Bauer, 2016/07/12 13:09:45]

Should this be a static_cast<int>? The style guide prefers those over C-style
casts, but then again this isn't really a cast at all... If static_cast<int>
isn't outright wrong here, I would go with that, I think, just for sake of being
consistent with the majority of Chrome code.

[Philipp Keck, 2016/07/12 13:45:02]

I delete this line anyway, but I use `int(...)` and `SomeEnumsName(someInt)`
elsewhere, so I could change those for consistency.

See also the discussion here:
https://codereview.chromium.org/2102023002/diff/80001/components/ntp_snippets...

[Bernhard Bauer, 2016/07/12 14:44:51]

Hm. I do prefer static_cast<>, but I don't want to reopen that discussion
either, so I would go with consistency and either change the other places, or
leave things as they are. 

[Philipp Keck, 2016/07/12 15:19:56]

Done. Actually the style guide is quite clear on this, I changed it to
static_cast<> on the other CL (the C-style cast was deleted here anyway).

+  }
+}
+
 } // namespace
 
 SnippetsInternalsMessageHandler::SnippetsInternalsMessageHandler()
-    : observer_(this),
+    : ntp_snippets_service_observer_(this),
+      content_suggestions_service_observer_(this),
       dom_loaded_(false),
-      ntp_snippets_service_(nullptr) {}
+      ntp_snippets_service_(nullptr),
+      content_suggestions_service_(nullptr) {}
 
 SnippetsInternalsMessageHandler::~SnippetsInternalsMessageHandler() {}
 
 void SnippetsInternalsMessageHandler::NTPSnippetsServiceShutdown() {}
 
 void SnippetsInternalsMessageHandler::NTPSnippetsServiceLoaded() {
   if (!dom_loaded_) return;
 
   SendSnippets();
 
   web_ui()->CallJavascriptFunctionUnsafe(
       "chrome.SnippetsInternals.receiveJson",
       base::StringValue(
           ntp_snippets_service_->snippets_fetcher()->last_json()));
 }
 
 void SnippetsInternalsMessageHandler::NTPSnippetsServiceDisabledReasonChanged(
     ntp_snippets::DisabledReason disabled_reason) {}
 
+void SnippetsInternalsMessageHandler::OnNewSuggestions() {
+  if (!dom_loaded_)
+    return;
+  SendContentSuggestions();
+}
+void SnippetsInternalsMessageHandler::OnCategoryStatusChanged(

[Marc Treib, 2016/07/12 13:04:53]

nit: newlines between methods please

[Philipp Keck, 2016/07/12 13:45:02]

Done.

+    ContentSuggestionsCategory category,
+    ContentSuggestionsCategoryStatus new_status) {
+  if (!dom_loaded_)
+    return;
+  SendContentSuggestions();
+}
+void SnippetsInternalsMessageHandler::ContentSuggestionsServiceShutdown() {}
+
 void SnippetsInternalsMessageHandler::RegisterMessages() {
   // additional initialization (web_ui() does not work from the constructor)
-  ntp_snippets_service_ = NTPSnippetsServiceFactory::GetInstance()->
-      GetForProfile(Profile::FromWebUI(web_ui()));
-  observer_.Add(ntp_snippets_service_);
+  Profile* profile = Profile::FromWebUI(web_ui());
+
+  ntp_snippets_service_ =
+      NTPSnippetsServiceFactory::GetInstance()->GetForProfile(profile);
+  ntp_snippets_service_observer_.Add(ntp_snippets_service_);
+
+  content_suggestions_service_ =
+      ContentSuggestionsServiceFactory::GetInstance()->GetForProfile(profile);
+  content_suggestions_service_observer_.Add(content_suggestions_service_);
 
   web_ui()->RegisterMessageCallback(
       "loaded",
       base::Bind(&SnippetsInternalsMessageHandler::HandleLoaded,
                  base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback(
       "clear", base::Bind(&SnippetsInternalsMessageHandler::HandleClear,
                           base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback(
       "download", base::Bind(&SnippetsInternalsMessageHandler::HandleDownload,
                              base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback(
       "clearDiscarded",
       base::Bind(&SnippetsInternalsMessageHandler::HandleClearDiscarded,
                  base::Unretained(this)));
+
+  web_ui()->RegisterMessageCallback(
+      "clearCachedSuggestions",
+      base::Bind(&SnippetsInternalsMessageHandler::HandleClearCachedSuggestions,
+                 base::Unretained(this)));
+
+  web_ui()->RegisterMessageCallback(
+      "clearDiscardedSuggestions",
+      base::Bind(
+          &SnippetsInternalsMessageHandler::HandleClearDiscardedSuggestions,
+          base::Unretained(this)));
 }
 
 void SnippetsInternalsMessageHandler::HandleLoaded(
     const base::ListValue* args) {
   DCHECK_EQ(0u, args->GetSize());
 
   dom_loaded_ = true;
 
   SendInitialData();
 }
@@ skipping 21 matching lines @@
   std::string hosts_string;
   args->GetString(0, &hosts_string);
 
   std::vector<std::string> hosts_vector = base::SplitString(
       hosts_string, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
   std::set<std::string> hosts(hosts_vector.begin(), hosts_vector.end());
 
   ntp_snippets_service_->FetchSnippetsFromHosts(hosts);
 }
 
+void SnippetsInternalsMessageHandler::HandleClearCachedSuggestions(
+    const base::ListValue* args) {
+  DCHECK_EQ(0u, args->GetSize());
+
+  content_suggestions_service_->ClearCachedSuggestionsForDebugging();
+}
+
+void SnippetsInternalsMessageHandler::HandleClearDiscardedSuggestions(
+    const base::ListValue* args) {
+  DCHECK_EQ(0u, args->GetSize());
+
+  content_suggestions_service_->ClearDiscardedSuggestionsForDebugging();
+}
+
 void SnippetsInternalsMessageHandler::SendInitialData() {
   SendHosts();
 
   SendBoolean("flag-snippets", base::FeatureList::IsEnabled(
                                    chrome::android::kNTPSnippetsFeature));
 
   web_ui()->CallJavascriptFunctionUnsafe(
       "chrome.SnippetsInternals.setHostRestricted",
       base::FundamentalValue(
           ntp_snippets_service_->snippets_fetcher()->UsesHostRestrictions()));
 
   switch (ntp_snippets_service_->snippets_fetcher()->personalization()) {
     case ntp_snippets::NTPSnippetsFetcher::Personalization::kPersonal:
       SendString("switch-personalized", "Only personalized");
       break;
     case ntp_snippets::NTPSnippetsFetcher::Personalization::kBoth:
       SendString("switch-personalized",
                  "Both personalized and non-personalized");
       break;
     case ntp_snippets::NTPSnippetsFetcher::Personalization::kNonPersonal:
       SendString("switch-personalized", "Only non-personalized");
       break;
   }
 
   SendString("switch-fetch-url",
              ntp_snippets_service_->snippets_fetcher()->fetch_url().spec());
 
   SendSnippets();
   SendDiscardedSnippets();
+  SendContentSuggestions();
 }
 
 void SnippetsInternalsMessageHandler::SendSnippets() {
   std::unique_ptr<base::ListValue> snippets_list(new base::ListValue);
 
   int index = 0;
   for (const std::unique_ptr<ntp_snippets::NTPSnippet>& snippet :
        ntp_snippets_service_->snippets())
     snippets_list->Append(PrepareSnippet(*snippet, index++, false));
 
@@ skipping 32 matching lines @@
 
     hosts_list->Append(std::move(entry));
   }
 
   base::DictionaryValue result;
   result.Set("list", std::move(hosts_list));
   web_ui()->CallJavascriptFunctionUnsafe(
       "chrome.SnippetsInternals.receiveHosts", result);
 }
 
+void SnippetsInternalsMessageHandler::SendContentSuggestions() {
+  std::unique_ptr<base::ListValue> categories_list(new base::ListValue);
+
+  int index = 0;
+  for (ContentSuggestionsCategory category :
+       content_suggestions_service_->GetCategories()) {
+    ContentSuggestionsCategoryStatus status =
+        content_suggestions_service_->GetCategoryStatus(category);
+    const std::vector<ContentSuggestion>& suggestions =
+        content_suggestions_service_->GetSuggestionsForCategory(category);
+
+    std::unique_ptr<base::ListValue> suggestions_list(new base::ListValue);
+    for (const ContentSuggestion& suggestion : suggestions) {
+      suggestions_list->Append(PrepareSuggestion(suggestion, index++));
+    }
+
+    std::unique_ptr<base::DictionaryValue> category_entry(
+        new base::DictionaryValue);
+    category_entry->SetString("name", MapCategoryName(category));
+    category_entry->SetString("status", MapCategoryStatus(status));
+    category_entry->Set("suggestions", std::move(suggestions_list));
+    categories_list->Append(std::move(category_entry));
+  }
+
+  base::DictionaryValue result;
+  result.Set("list", std::move(categories_list));
+  web_ui()->CallJavascriptFunctionUnsafe(
+      "chrome.SnippetsInternals.receiveContentSuggestions", result);
+}
+
 void SnippetsInternalsMessageHandler::SendBoolean(const std::string& name,
                                                   bool value) {
   SendString(name, value ? "True" : "False");
 }
 
 void SnippetsInternalsMessageHandler::SendString(const std::string& name,
                                                  const std::string& value) {
   base::StringValue string_name(name);
   base::StringValue string_value(value);
 
   web_ui()->CallJavascriptFunctionUnsafe(
       "chrome.SnippetsInternals.receiveProperty", string_name, string_value);
 }