Extend LargeIconService to fetch missing favicons from a Google server

components/favicon/core/BUILD.gn

Index: components/favicon/core/BUILD.gn
diff --git a/components/favicon/core/BUILD.gn b/components/favicon/core/BUILD.gn
index c086a6c910dd17bcd5ecef96aaf3f3fb334594b9..794f26c1888b91ce8878ac4a1e0cc2ee6f733981 100644
--- a/components/favicon/core/BUILD.gn
+++ b/components/favicon/core/BUILD.gn
@@ -26,13 +26,18 @@ static_library("core") {
     "large_icon_service.h",
   ]
 
-  deps = [
+  public_deps = [

[sdefresne, 2017/03/23 10:24:41]

I don't think this whole public_deps is correct. It looks like we generally do
not declare public_deps on //base or other targets just because there are
#include of the target in headers.

It looks like they are used either when trying to masquerade a collection of
targets appear like a single target (usually in templates) or when one of the
depended-on target has public_configs.

At least, it appears to be incorrect for //base (it is not part of
//components/favicon/core and does not export public_configs), and probably for
the other targets too for the same reason. I think they should stay in deps and
instead the dependencies fixed in the client code.

[mastiz, 2017/03/23 12:03:23]

Done.

Although not many, there are certainly examples of //base in public_deps,
introduced by senior engineers, e.g.:
https://cs.chromium.org/chromium/src/gin/BUILD.gn?type=cs&q=public_deps&l=98

From the official documentation
(https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/referen...):

"Generally if you are writing a target B and you include C's headers as part of
B's public headers, or targets depending on B should consider B and C to be part
of a unit, you should use public_deps instead of deps."

My interpretation is that the first condition is sufficient (i.e. "you include
C's headers as part of B's public headers"), to use public_deps.

Having said that, while searching for information, I came across one of your
patches:
https://r882-11cdd11fc1da-maruel-dot-chromiumcodereview.appspot.com/2539143002/

Which suggests you've thought this through, so addressed your suggestion.

     "//base",
+    "//components/favicon_base",
+    "//components/image_fetcher/core",
+    "//components/keyed_service/core",
+  ]
+
+  deps = [
     "//base:i18n",
     "//components/bookmarks/browser",
-    "//components/favicon_base",
+    "//components/data_use_measurement/core",
     "//components/history/core/browser",
-    "//components/keyed_service/core",
     "//net:net",
     "//skia",
     "//ui/base",
@@ -66,6 +71,7 @@ source_set("unit_tests") {
     "//components/favicon_base",
     "//components/history/core/browser:browser",
     "//components/history/core/test:test",
+    "//components/image_fetcher/core",
     "//skia",
     "//testing/gmock",
     "//testing/gtest",