ntp_tiles: Extend chrome://ntp-tiles-internals with favicon data

components/ntp_tiles/webui/ntp_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/ntp_tiles_internals_message_handler.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/files/file_util.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/task_runner_util.h"
 #include "base/values.h"
+#include "components/favicon/core/favicon_service.h"
 #include "components/ntp_tiles/most_visited_sites.h"
 #include "components/ntp_tiles/pref_names.h"
 #include "components/ntp_tiles/webui/ntp_tiles_internals_message_handler_client.h"
 #include "components/prefs/pref_service.h"
 #include "components/url_formatter/url_fixer.h"
 #include "url/gurl.h"
 
 namespace ntp_tiles {
 
 namespace {
 
 std::string FormatJson(const base::Value& value) {
   std::string pretty_printed;
   bool ok = base::JSONWriter::WriteWithOptions(
       value, base::JSONWriter::OPTIONS_PRETTY_PRINT, &pretty_printed);
   DCHECK(ok);
   return pretty_printed;
 }
 
 }  // namespace
 
-NTPTilesInternalsMessageHandler::NTPTilesInternalsMessageHandler()
-    : client_(nullptr), site_count_(8), weak_ptr_factory_(this) {}
+NTPTilesInternalsMessageHandler::NTPTilesInternalsMessageHandler(
+    favicon::FaviconService* favicon_service)
+    : favicon_service_(favicon_service),
+      icon_types_and_names_{

[sfiera, 2017/06/13 08:56:01]

This looks like it could be a constexpr std::array<std::pair<>> local to the .cc
file.

[mastiz, 2017/06/13 11:19:39]

Done.

+          {favicon_base::FAVICON, "FAVICON"},
+          {favicon_base::TOUCH_ICON, "TOUCH_ICON"},
+          {favicon_base::TOUCH_PRECOMPOSED_ICON, "TOUCH_PRECOMPOSED_ICON"},
+          {favicon_base::WEB_MANIFEST_ICON, "WEB_MANIFEST_ICON"},
+      },
+      client_(nullptr),
+      site_count_(8),
+      weak_ptr_factory_(this) {}
 
 NTPTilesInternalsMessageHandler::~NTPTilesInternalsMessageHandler() = default;
 
 void NTPTilesInternalsMessageHandler::RegisterMessages(
     NTPTilesInternalsMessageHandlerClient* client) {
   client_ = client;
 
   client_->RegisterMessageCallback(
       "registerForEvents",
       base::Bind(&NTPTilesInternalsMessageHandler::HandleRegisterForEvents,
@@ skipping 17 matching lines @@
 void NTPTilesInternalsMessageHandler::HandleRegisterForEvents(
     const base::ListValue* args) {
   if (!client_->SupportsNTPTiles()) {
     base::DictionaryValue disabled;
     disabled.SetBoolean("topSites", false);
     disabled.SetBoolean("suggestionsService", false);
     disabled.SetBoolean("popular", false);
     disabled.SetBoolean("whitelist", false);
     client_->CallJavascriptFunction(
         "chrome.ntp_tiles_internals.receiveSourceInfo", disabled);
-    SendTiles(NTPTilesVector());
+    SendTiles(NTPTilesVector(), FaviconResultMap());
     return;
   }
   DCHECK(args->empty());
 
   suggestions_status_.clear();
   popular_sites_json_.clear();
   most_visited_sites_ = client_->MakeMostVisitedSites();
   most_visited_sites_->SetMostVisitedURLsObserver(this, site_count_);
   SendSourceInfo();
 }
@@ skipping 122 matching lines @@
 
     value.SetString("popular.json", popular_sites_json_);
   } else {
     value.SetBoolean("popular", false);
   }
 
   client_->CallJavascriptFunction(
       "chrome.ntp_tiles_internals.receiveSourceInfo", value);
 }
 
-void NTPTilesInternalsMessageHandler::SendTiles(const NTPTilesVector& tiles) {
+void NTPTilesInternalsMessageHandler::SendTiles(
+    const NTPTilesVector& tiles,
+    const FaviconResultMap& result_map) {
   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));
     entry->SetString("whitelistIconPath",
                      tile.whitelist_icon_path.LossyDisplayName());
+
+    auto icon_list = base::MakeUnique<base::ListValue>();
+    for (const auto& icon_type : icon_types_and_names_) {
+      FaviconResultMap::const_iterator it = result_map.find(
+          FaviconResultMap::key_type(tile.url, icon_type.first));
+
+      if (it != result_map.end()) {
+        auto icon = base::MakeUnique<base::DictionaryValue>();
+        icon->SetString("url", it->second.icon_url.spec());
+        icon->SetString("type", icon_type.second);
+        icon_list->Append(std::move(icon));
+      }
+    }
+    entry->Set("icons", std::move(icon_list));
+
     sites_list->Append(std::move(entry));
   }
 
   base::DictionaryValue result;
   result.Set("sites", std::move(sites_list));
   client_->CallJavascriptFunction("chrome.ntp_tiles_internals.receiveSites",
                                   result);
 }
 
 void NTPTilesInternalsMessageHandler::OnMostVisitedURLsAvailable(
     const NTPTilesVector& tiles) {

[sfiera, 2017/06/13 08:56:01]

First, cancelable_task_tracker_.CancelAll() in case we're still trying to load
icons.

[mastiz, 2017/06/13 11:19:39]

Done.

-  SendTiles(tiles);
+  if (tiles.empty())

[sfiera, 2017/06/13 08:56:00]

Nit: braces
And early return?

[mastiz, 2017/06/13 11:19:39]

Done.

+    SendTiles(tiles, FaviconResultMap());
+
+  auto on_lookup_done = base::Bind(

[sfiera, 2017/06/13 08:56:01]

BindRepeating() to make the usage clear?

[mastiz, 2017/06/13 11:19:39]

Done.

+      &NTPTilesInternalsMessageHandler::OnFaviconLookupDone,
+      base::Unretained(this), tiles, base::Owned(new FaviconResultMap()),

[sfiera, 2017/06/13 08:56:01]

Can you add a comment about Unretained? It's safe because of
|cancelable_task_tracker_|, right?

[mastiz, 2017/06/13 11:19:39]

Done. However, I fail to see why this is different to using |weak_ptr_factory_|
which is not documented similarly.

[sfiera, 2017/06/13 11:34:13]

Whenever I see Unretained(), I wonder what mechanism allows it to be safe. When
I see weak_ptr_factory_, I know the mechanism.

+      base::Owned(new size_t(tiles.size() * icon_types_and_names_.size())));
+
+  for (const NTPTile& tile : tiles) {
+    for (const auto& icon_type : icon_types_and_names_) {
+      favicon_service_->GetLargestRawFaviconForPageURL(
+          tile.url, std::vector<int>(1U, icon_type.first),
+          /*minimum_size_in_pixels=*/0, base::Bind(on_lookup_done, tile.url),
+          &cancelable_task_tracker_);
+    }
+  }
 }
 
 void NTPTilesInternalsMessageHandler::OnIconMadeAvailable(
     const GURL& site_url) {}
 
+void NTPTilesInternalsMessageHandler::OnFaviconLookupDone(
+    const NTPTilesVector& tiles,
+    FaviconResultMap* result_map,
+    size_t* num_pending_lookups,
+    const GURL& page_url,
+    const favicon_base::FaviconRawBitmapResult& result) {
+  DCHECK_NE(0, *num_pending_lookups);
+
+  result_map->emplace(
+      std::pair<GURL, favicon_base::IconType>(page_url, result.icon_type),
+      result);
+
+  --*num_pending_lookups;
+  if (*num_pending_lookups == 0)
+    SendTiles(tiles, *result_map);
+}
+
 }  // namespace ntp_tiles