Add chrome://site-tiles-internals/

components/ntp_tiles/webui/site_tiles_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 "components/ntp_tiles/webui/site_tiles_internals_message_handler.h"
+
+#include "base/bind.h"
+#include "base/callback.h"
+#include "base/files/file_util.h"
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+#include "base/task_runner_util.h"
+#include "base/values.h"
+#include "components/ntp_tiles/most_visited_sites.h"
+#include "components/ntp_tiles/pref_names.h"
+#include "components/ntp_tiles/webui/site_tiles_internals_message_handler_client.h"
+#include "components/prefs/pref_service.h"
+#include "components/url_formatter/url_fixer.h"
+#include "url/gurl.h"
+
+namespace {
+
+std::string ReadFileToString(const base::FilePath& path) {
+  std::string result;
+  if (!base::ReadFileToString(path, &result))
+    result.clear();

[Marc Treib, 2016/12/08 15:41:50]

nit: Does/should our new "always use braces" rule apply here too?

[sfiera, 2016/12/08 16:44:35]

Yes, I think so. I copy-pasted this function, but it's not actually used now
because of the removal of viewing the JSON, so removed.

+  return result;
+}
+
+}  // namespace
+
+namespace ntp_tiles {
+
+SiteTilesInternalsMessageHandlerClient::
+    SiteTilesInternalsMessageHandlerClient() = default;
+SiteTilesInternalsMessageHandlerClient::
+    ~SiteTilesInternalsMessageHandlerClient() = default;
+
+SiteTilesInternalsMessageHandler::SiteTilesInternalsMessageHandler(
+    SiteTilesInternalsMessageHandlerClient* web_ui)
+    : web_ui_(web_ui), site_count_(8), weak_ptr_factory_(this) {}

[Marc Treib, 2016/12/08 15:41:50]

Any plans to make site_count_ non-constant? If not, just make it a kSiteCount
constant?

[sfiera, 2016/12/08 16:44:35]

It should have that feature, since we sometimes fetch 12 (Android, non-Google
DSE). I didn't consider it worth more effort than this yet, though.

[Marc Treib, 2016/12/08 17:33:22]

I'd argue that as long as it doesn't have that feature, site_count shouldn't be
a member :)
But okay, fair enough.

+
+SiteTilesInternalsMessageHandler::~SiteTilesInternalsMessageHandler() = default;
+
+void SiteTilesInternalsMessageHandler::RegisterMessages() {
+  web_ui_->RegisterMessageCallback(
+      "registerForEvents",
+      base::Bind(&SiteTilesInternalsMessageHandler::HandleRegisterForEvents,
+                 base::Unretained(this)));
+
+  web_ui_->RegisterMessageCallback(
+      "update", base::Bind(&SiteTilesInternalsMessageHandler::HandleUpdate,
+                           base::Unretained(this)));
+
+  web_ui_->RegisterMessageCallback(
+      "viewPopularSitesJson",
+      base::Bind(&SiteTilesInternalsMessageHandler::HandleViewPopularSitesJson,
+                 base::Unretained(this)));
+
+  web_ui_->RegisterMessageCallback(
+      "getFavicon",
+      base::Bind(&SiteTilesInternalsMessageHandler::HandleGetFavicon,
+                 base::Unretained(this)));
+}
+
+void SiteTilesInternalsMessageHandler::HandleRegisterForEvents(
+    const base::ListValue* args) {
+  DCHECK(args->empty());
+
+  SendSourceInfo();
+
+  most_visited_sites_ = web_ui_->MakeMostVisitedSites();
+  most_visited_sites_->SetMostVisitedURLsObserver(this, site_count_);
+}
+
+void SiteTilesInternalsMessageHandler::HandleUpdate(
+    const base::ListValue* args) {
+  const base::DictionaryValue* dict;
+  DCHECK_EQ(1u, args->GetSize());
+  DCHECK(args->GetDictionary(0, &dict));
+
+  PrefService* prefs = web_ui_->GetPrefs();
+
+  if (web_ui_->IsSourceEnabled(ntp_tiles::NTPTileSource::POPULAR)) {
+    std::string url;
+    dict->GetString("popular.overrideURL", &url);
+    if (url.empty())
+      prefs->ClearPref(ntp_tiles::prefs::kPopularSitesOverrideURL);
+    else
+      prefs->SetString(ntp_tiles::prefs::kPopularSitesOverrideURL,
+                       url_formatter::FixupURL(url, std::string()).spec());
+
+    std::string country;
+    dict->GetString("popular.overrideCountry", &country);
+    if (country.empty())
+      prefs->ClearPref(ntp_tiles::prefs::kPopularSitesOverrideCountry);
+    else
+      prefs->SetString(ntp_tiles::prefs::kPopularSitesOverrideCountry, country);
+
+    std::string version;
+    dict->GetString("popular.overrideVersion", &version);
+    if (version.empty())
+      prefs->ClearPref(ntp_tiles::prefs::kPopularSitesOverrideVersion);
+    else
+      prefs->SetString(ntp_tiles::prefs::kPopularSitesOverrideVersion, version);
+  }
+
+  // Recreate to pick up new values.
+  most_visited_sites_ = web_ui_->MakeMostVisitedSites();
+  most_visited_sites_->SetMostVisitedURLsObserver(this, site_count_);
+  SendSourceInfo();
+}
+
+void SiteTilesInternalsMessageHandler::HandleViewPopularSitesJson(
+    const base::ListValue* args) {
+  DCHECK_EQ(0u, args->GetSize());
+
+  const base::FilePath& path = web_ui_->MakePopularSites()->local_path();

[Marc Treib, 2016/12/08 15:41:50]

Should we maybe expose the PopularSites instance from MostVisitedSites? Then
there wouldn't be so many instances of it flying around, and I think
MakePopularSites would become unnecessary.

[sfiera, 2016/12/08 16:44:35]

Well, the alternate plan is to make PopularSites long-lived, so we don't need to
create it and read a file every time the NTP is opened. I'd rather not widen
MostVisitedSites' interface unnecessarily, and I'd rather not put in the work to
change PopularSites, so this was an easy middle ground :)

[Marc Treib, 2016/12/08 17:33:22]

Well, the alternate alternate plan is to make MostVisitedSites long-lived
(that's how it'll be used on desktop anyway). Then it could continue to own
PopularSites, and that would be the one and only instance.

+  base::PostTaskAndReplyWithResult(
+      web_ui_->GetBlockingPool()
+          ->GetTaskRunnerWithShutdownBehavior(
+              base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN)
+          .get(),
+      FROM_HERE, base::Bind(&ReadFileToString, path),
+      base::Bind(&SiteTilesInternalsMessageHandler::SendPopularSitesJson,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void SiteTilesInternalsMessageHandler::HandleGetFavicon(
+    const base::ListValue* args) {
+  DCHECK_EQ(1u, args->GetSize());
+  // TODO(sfiera): implement
+}
+
+void SiteTilesInternalsMessageHandler::SendSourceInfo() {
+  PrefService* prefs = web_ui_->GetPrefs();
+  base::DictionaryValue value;
+
+  value.SetBoolean("topSites",
+                   web_ui_->IsSourceEnabled(NTPTileSource::TOP_SITES));
+  value.SetBoolean(
+      "suggestionsService",
+      web_ui_->IsSourceEnabled(NTPTileSource::SUGGESTIONS_SERVICE));
+  value.SetBoolean("whitelist",
+                   web_ui_->IsSourceEnabled(NTPTileSource::WHITELIST));
+
+  if (web_ui_->IsSourceEnabled(NTPTileSource::POPULAR)) {
+    auto popular_sites = web_ui_->MakePopularSites();
+    value.SetString("popular.url", popular_sites->GetURLToUse().spec());
+    value.SetString("popular.country", popular_sites->GetCountryToUse());
+    value.SetString("popular.version", popular_sites->GetVersionToUse());
+
+    value.SetString(
+        "popular.overrideURL",
+        prefs->GetString(ntp_tiles::prefs::kPopularSitesOverrideURL));
+    value.SetString(
+        "popular.overrideCountry",
+        prefs->GetString(ntp_tiles::prefs::kPopularSitesOverrideCountry));
+    value.SetString(
+        "popular.overrideVersion",
+        prefs->GetString(ntp_tiles::prefs::kPopularSitesOverrideVersion));
+  } else {
+    value.SetBoolean("popular", false);

[Marc Treib, 2016/12/08 15:41:50]

It doesn't need to be set to true in the enabled case?

[sfiera, 2016/12/08 16:44:35]

It is true in the enabled case, because objects are true :)

[Marc Treib, 2016/12/08 17:33:22]

Ewwww. Oh well.

[sfiera, 2016/12/08 18:00:48]

(I suppose, if you want, the state could be {
  "topSites": {"enabled": true},
  "suggestionsService": {"enabled": true},
  "popular": {"enabled": false},
  "whitelist": {"enabled": false},
})

[Marc Treib, 2016/12/09 10:52:11]

Naaah, it's okay. I just don't like JS :)

+  }
+
+  web_ui_->CallJavascriptFunction(
+      "chrome.site_tiles_internals.receiveSourceInfo", value);
+}
+
+void SiteTilesInternalsMessageHandler::SendTiles(const NTPTilesVector& tiles) {
+  auto sites_list = base::MakeUnique<base::ListValue>();
+  for (const NTPTile& tile : tiles) {
+    auto entry = base::MakeUnique<base::DictionaryValue>();
+    entry->SetString("title", tile.title);
+    entry->SetString("url", tile.url.spec());
+    entry->SetInteger("source", static_cast<int>(tile.source));
+    if (tile.source == NTPTileSource::WHITELIST) {
+      entry->SetString("whitelistIconPath",
+                       tile.whitelist_icon_path.AsUTF8Unsafe());

[Marc Treib, 2016/12/08 15:41:50]

Huh, contrary to what the comment on NTPTile says, this can actually be set for
other sources too. So maybe always set it (and remove/update that comment)?

Also, maybe LossyDisplayName rather than AsUTF8Unsafe?

[sfiera, 2016/12/08 16:44:35]

Done, not done, and done. (set for other sources, update comment, use
LossyDisplayName)

I'll update the comment separately.

+    }
+    sites_list->Append(std::move(entry));
+  }
+
+  base::DictionaryValue result;
+  result.Set("sites", std::move(sites_list));
+  web_ui_->CallJavascriptFunction("chrome.site_tiles_internals.receiveSites",
+                                  result);
+}
+
+void SiteTilesInternalsMessageHandler::SendPopularSitesJson(
+    const std::string& json) {
+  web_ui_->CallJavascriptFunction(
+      "chrome.site_tiles_internals.receivePopularSitesJson",
+      base::StringValue(json));
+}
+
+void SiteTilesInternalsMessageHandler::OnMostVisitedURLsAvailable(
+    const NTPTilesVector& tiles) {
+  SendTiles(tiles);
+}
+
+void SiteTilesInternalsMessageHandler::OnIconMadeAvailable(
+    const GURL& site_url) {}
+
+}  // namespace ntp_tiles