Add baked-in favicons for default popular sites on NTP

components/ntp_tiles/popular_sites_impl.cc

 // Copyright 2015 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/popular_sites_impl.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 111 matching lines @@
       continue;
     std::string favicon_url;
     item->GetString("favicon_url", &favicon_url);
     std::string thumbnail_url;
     item->GetString("thumbnail_url", &thumbnail_url);
     std::string large_icon_url;
     item->GetString("large_icon_url", &large_icon_url);
 
     sites.emplace_back(title, GURL(url), GURL(favicon_url),
                        GURL(large_icon_url), GURL(thumbnail_url));
+    item->GetInteger("default_resource_id", &sites.back().default_resource_id);
   }
   return sites;
 }
 
+#if defined(OS_ANDROID) || defined(OS_IOS)
+void SetDefaultResourceForSite(base::ListValue* sites,
+                               int index,
+                               int resource_id) {
+  base::DictionaryValue* site;
+  if (!sites->GetDictionary(index, &site)) {
+    return;
+  }
+  site->SetInteger("default_resource_id", resource_id);

[sfiera, 2017/02/16 18:54:16]

Feels a little weird to be storing a resource ID in preferences. But I guess
it's not really in the preferences, just an ephemeral default value…

[fhorschig, 2017/02/17 16:24:04]

Yes. With the first real fetch, this value is gone.
I had the same thought but adding this property apart from the
definition of the default popular sites makes for some huge exceptions
and likely pitfalls.

+}
+#endif
+
 // Creates the list of popular sites based on a snapshot available for mobile.
 std::unique_ptr<base::ListValue> DefaultPopularSites() {
 #if defined(OS_ANDROID) || defined(OS_IOS)
   std::unique_ptr<base::ListValue> sites =
       base::ListValue::From(base::JSONReader().ReadToValue(
           ResourceBundle::GetSharedInstance().GetRawDataResource(
               IDR_DEFAULT_POPULAR_SITES_JSON)));
   DCHECK(sites);
+  SetDefaultResourceForSite(sites.get(), 0, IDR_DEFAULT_POPULAR_SITES_ICON0);

[sfiera, 2017/02/16 18:54:16]

Couldn't you write this as a foreach loop? Like:

  int index = 0;
  for (int resource : {IDR_DEFAULT_POPULAR_SITES_ICON0,
                       …}) {

[fhorschig, 2017/02/17 16:24:04]

YES! This is exactly what I have been looking for. Thanks!

+  SetDefaultResourceForSite(sites.get(), 1, IDR_DEFAULT_POPULAR_SITES_ICON1);
+  SetDefaultResourceForSite(sites.get(), 2, IDR_DEFAULT_POPULAR_SITES_ICON2);
+  SetDefaultResourceForSite(sites.get(), 3, IDR_DEFAULT_POPULAR_SITES_ICON3);
+  SetDefaultResourceForSite(sites.get(), 4, IDR_DEFAULT_POPULAR_SITES_ICON4);
+  SetDefaultResourceForSite(sites.get(), 5, IDR_DEFAULT_POPULAR_SITES_ICON5);
+  SetDefaultResourceForSite(sites.get(), 6, IDR_DEFAULT_POPULAR_SITES_ICON6);
+  SetDefaultResourceForSite(sites.get(), 7, IDR_DEFAULT_POPULAR_SITES_ICON7);
   return sites;
 #endif
   return base::MakeUnique<base::ListValue>();
 }
 
 }  // namespace
 
 PopularSites::Site::Site(const base::string16& title,
                          const GURL& url,
                          const GURL& favicon_url,
                          const GURL& large_icon_url,
                          const GURL& thumbnail_url)
     : title(title),
       url(url),
       favicon_url(favicon_url),
       large_icon_url(large_icon_url),
-      thumbnail_url(thumbnail_url) {}
+      thumbnail_url(thumbnail_url),
+      default_resource_id(-1) {}  // Valid resources would be >=0

[sfiera, 2017/02/16 18:54:16]

I don't think any comment is needed in addition to the header.

[fhorschig, 2017/02/17 16:24:04]

Done.

 
 PopularSites::Site::Site(const Site& other) = default;
 
 PopularSites::Site::~Site() {}
 
 PopularSitesImpl::PopularSitesImpl(
     const scoped_refptr<base::SequencedWorkerPool>& blocking_pool,
     PrefService* prefs,
     const TemplateURLService* template_url_service,
     VariationsService* variations_service,
@@ skipping 193 matching lines @@
     pending_url_ = GetPopularSitesURL(kPopularSitesDefaultCountryCode,
                                       kPopularSitesDefaultVersion);
     FetchPopularSites();
   } else {
     DLOG(WARNING) << "Download fallback site list failed";
     callback_.Run(false);
   }
 }
 
 }  // namespace ntp_tiles