fed34be47fbaa165e6d0817f55ae907a97ea00e8 - chromium/src

[fhor...@chromium.org]

commit fed34be47fbaa165e6d0817f55ae907a97ea00e8
Author: fhorschig <fhor...@chromium.org>
AuthorDate: Thu Mar 02 23:16:09 2017
Commit: Commit bot <commi...@chromium.org>
CommitDate: Thu Mar 02 23:16:09 2017

Add access to baked-in favicons for default popular sites on NTP

This change enables branded builds to use icons for the first
opened New Tab Page by adding them to the initial icon cache.

Non-branded builds receive a new set of popular site tiles.
(This profits developers mainly.)

BUG=672939

Review-Url: https://codereview.chromium.org/2695713004
Cr-Commit-Position: refs/heads/master@{#454424}

diff --git a/components/ntp_tiles/BUILD.gn b/components/ntp_tiles/BUILD.gn
index 050f8a5..0e98bf7 100644
--- a/components/ntp_tiles/BUILD.gn
+++ b/components/ntp_tiles/BUILD.gn
@@ -104,6 +104,7 @@
"//net:test_support",
"//testing/gmock",
"//testing/gtest",
+ "//ui/base",
"//ui/gfx:test_support",
]
}
diff --git a/components/ntp_tiles/DEPS b/components/ntp_tiles/DEPS
index 2296e56..cf31b04 100644
--- a/components/ntp_tiles/DEPS
+++ b/components/ntp_tiles/DEPS
@@ -17,5 +17,6 @@
"+jni",
"+net",
"+ui/gfx",
- "+ui/base/resource",
+ "+ui/base",
]
+
diff --git a/components/ntp_tiles/icon_cacher.h b/components/ntp_tiles/icon_cacher.h
index 22beeb6..e7456fd 100644
--- a/components/ntp_tiles/icon_cacher.h
+++ b/components/ntp_tiles/icon_cacher.h
@@ -20,10 +20,14 @@
public:
virtual ~IconCacher() = default;

- // Fetches the icon if necessary, then invokes |done| with true if it was
- // newly fetched (false if it was already cached or could not be fetched).
+ // Fetches the icon if necessary. It invokes |icon_available| if a new icon
+ // was fetched. This is the only required callback.
+ // If there are preliminary icons (e.g. provided by static resources), the
+ // |preliminary_icon_available| callback will be invoked additionally.
+ // TODO(fhorschig): In case we keep these, make them OnceClosures.
virtual void StartFetch(PopularSites::Site site,
- const base::Callback<void(bool)>& done) = 0;
+ const base::Closure& icon_available,
+ const base::Closure& preliminary_icon_available) = 0;
};

} // namespace ntp_tiles
diff --git a/components/ntp_tiles/icon_cacher_impl.cc b/components/ntp_tiles/icon_cacher_impl.cc
index c5c85ac..aa4e911 100644
--- a/components/ntp_tiles/icon_cacher_impl.cc
+++ b/components/ntp_tiles/icon_cacher_impl.cc
@@ -11,6 +11,7 @@
#include "components/favicon_base/favicon_types.h"
#include "components/favicon_base/favicon_util.h"
#include "components/image_fetcher/image_fetcher.h"
+#include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h"
#include "url/gurl.h"
@@ -44,43 +45,62 @@

IconCacherImpl::~IconCacherImpl() = default;

-void IconCacherImpl::StartFetch(PopularSites::Site site,
- const base::Callback<void(bool)>& done) {
+void IconCacherImpl::StartFetch(
+ PopularSites::Site site,
+ const base::Closure& icon_available,
+ const base::Closure& preliminary_icon_available) {
+ DCHECK(!icon_available.is_null());
favicon::GetFaviconImageForPageURL(
favicon_service_, site.url, IconType(site),
base::Bind(&IconCacherImpl::OnGetFaviconImageForPageURLFinished,
- base::Unretained(this), std::move(site), done),
+ base::Unretained(this), std::move(site), icon_available,
+ preliminary_icon_available),
&tracker_);
}

void IconCacherImpl::OnGetFaviconImageForPageURLFinished(
PopularSites::Site site,
- const base::Callback<void(bool)>& done,
+ const base::Closure& icon_available,
+ const base::Closure& preliminary_icon_available,
const favicon_base::FaviconImageResult& result) {
if (!result.image.IsEmpty()) {
- done.Run(false);
return;
+ }
+ if (ProvideDefaultIcon(site) && !preliminary_icon_available.is_null()) {
+ preliminary_icon_available.Run();
}

image_fetcher_->StartOrQueueNetworkRequest(
std::string(), IconURL(site),
base::Bind(&IconCacherImpl::OnFaviconDownloaded, base::Unretained(this),
- site, done));
+ site, icon_available));
}

void IconCacherImpl::OnFaviconDownloaded(PopularSites::Site site,
- const base::Callback<void(bool)>& done,
+ const base::Closure& icon_available,
const std::string& id,
const gfx::Image& fetched_image) {
if (fetched_image.IsEmpty()) {
- done.Run(false);
return;
}

- gfx::Image image = fetched_image;
+ SaveIconForSite(site, fetched_image);
+ icon_available.Run();
+}
+
+void IconCacherImpl::SaveIconForSite(const PopularSites::Site& site,
+ gfx::Image image) {
favicon_base::SetFaviconColorSpace(&image);
favicon_service_->SetFavicons(site.url, IconURL(site), IconType(site), image);
- done.Run(true);
+}
+
+bool IconCacherImpl::ProvideDefaultIcon(const PopularSites::Site& site) {
+ if (site.default_icon_resource < 0) {
+ return false;
+ }
+ SaveIconForSite(site, ResourceBundle::GetSharedInstance().GetNativeImageNamed(
+ site.default_icon_resource));
+ return true;
}

} // namespace ntp_tiles
diff --git a/components/ntp_tiles/icon_cacher_impl.h b/components/ntp_tiles/icon_cacher_impl.h
index 1028f72..20c5922 100644
--- a/components/ntp_tiles/icon_cacher_impl.h
+++ b/components/ntp_tiles/icon_cacher_impl.h
@@ -39,19 +39,24 @@
~IconCacherImpl() override;

void StartFetch(PopularSites::Site site,
- const base::Callback<void(bool)>& done) override;
+ const base::Closure& icon_available,
+ const base::Closure& preliminary_icon_available) override;

private:
void OnGetFaviconImageForPageURLFinished(
PopularSites::Site site,
- const base::Callback<void(bool)>& done,
+ const base::Closure& icon_available,
+ const base::Closure& preliminary_icon_available,
const favicon_base::FaviconImageResult& result);

void OnFaviconDownloaded(PopularSites::Site site,
- const base::Callback<void(bool)>& done,
+ const base::Closure& icon_available,
const std::string& id,
const gfx::Image& fetched_image);

+ bool ProvideDefaultIcon(const PopularSites::Site& site);
+ void SaveIconForSite(const PopularSites::Site& site, const gfx::Image image);
+
base::CancelableTaskTracker tracker_;
favicon::FaviconService* const favicon_service_;
std::unique_ptr<image_fetcher::ImageFetcher> const image_fetcher_;
diff --git a/components/ntp_tiles/icon_cacher_impl_unittest.cc b/components/ntp_tiles/icon_cacher_impl_unittest.cc
index eb7ff45..721aa00 100644
--- a/components/ntp_tiles/icon_cacher_impl_unittest.cc
+++ b/components/ntp_tiles/icon_cacher_impl_unittest.cc
@@ -4,11 +4,15 @@

#include "components/ntp_tiles/icon_cacher_impl.h"

+#include <set>
#include <utility>

#include "base/files/scoped_temp_dir.h"
#include "base/memory/ptr_util.h"
+#include "base/path_service.h"
#include "base/run_loop.h"
+#include "base/test/mock_callback.h"
+#include "base/test/test_simple_task_runner.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/favicon/core/favicon_client.h"
#include "components/favicon/core/favicon_service_impl.h"
@@ -18,12 +22,17 @@
#include "components/image_fetcher/image_fetcher.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/base/ui_base_paths.h"
#include "ui/gfx/image/image_unittest_util.h"

using ::testing::_;
+using ::testing::Eq;
+using ::testing::Invoke;
using ::testing::InSequence;
-using ::testing::MockFunction;
+using ::testing::NiceMock;
using ::testing::Return;
+using ::testing::ReturnArg;

namespace ntp_tiles {
namespace {
@@ -42,6 +51,36 @@
MOCK_METHOD1(SetDesiredImageFrameSize, void(const gfx::Size&));
};

+// This class provides methods to inject an image resource where a real resource
+// would be necessary otherwise. All other methods have return values that allow
+// the normal implementation to proceed.
+class MockResourceDelegate : public ui::ResourceBundle::Delegate {
+ public:
+ ~MockResourceDelegate() override {}
+
+ MOCK_METHOD1(GetImageNamed, gfx::Image(int resource_id));
+ MOCK_METHOD1(GetNativeImageNamed, gfx::Image(int resource_id));
+
+ MOCK_METHOD2(GetPathForResourcePack,
+ base::FilePath(const base::FilePath& pack_path,
+ ui::ScaleFactor scale_factor));
+
+ MOCK_METHOD2(GetPathForLocalePack,
+ base::FilePath(const base::FilePath& pack_path,
+ const std::string& locale));
+
+ MOCK_METHOD2(LoadDataResourceBytes,
+ base::RefCountedMemory*(int resource_id,
+ ui::ScaleFactor scale_factor));
+
+ MOCK_METHOD3(GetRawDataResource,
+ bool(int resource_id,
+ ui::ScaleFactor scale_factor,
+ base::StringPiece* value));
+
+ MOCK_METHOD2(GetLocalizedString, bool(int message_id, base::string16* value));
+};
+
class IconCacherTest : public ::testing::Test {
protected:
IconCacherTest()
@@ -51,10 +90,44 @@
GURL("http://url.google/favicon.ico"),
GURL()), // thumbnail, unused
image_fetcher_(new ::testing::StrictMock<MockImageFetcher>),
- favicon_service_(/*favicon_client=*/nullptr, &history_service_) {
+ favicon_service_(/*favicon_client=*/nullptr, &history_service_),
+ task_runner_(new base::TestSimpleTaskRunner()) {
CHECK(history_dir_.CreateUniqueTempDir());
CHECK(history_service_.Init(
history::HistoryDatabaseParams(history_dir_.GetPath(), 0, 0)));
+ }
+
+ void SetUp() override {
+ if (ui::ResourceBundle::HasSharedInstance()) {
+ ui::ResourceBundle::CleanupSharedInstance();
+ }
+ ON_CALL(mock_resource_delegate_, GetPathForResourcePack(_, _))
+ .WillByDefault(ReturnArg<0>());
+ ON_CALL(mock_resource_delegate_, GetPathForLocalePack(_, _))
+ .WillByDefault(ReturnArg<0>());
+ ui::ResourceBundle::InitSharedInstanceWithLocale(
+ "en-US", &mock_resource_delegate_,
+ ui::ResourceBundle::LOAD_COMMON_RESOURCES);
+ }
+
+ void TearDown() override {
+ if (ui::ResourceBundle::HasSharedInstance()) {
+ ui::ResourceBundle::CleanupSharedInstance();
+ }
+ base::FilePath pak_path;
+#if defined(OS_ANDROID)
+ PathService::Get(ui::DIR_RESOURCE_PAKS_ANDROID, &pak_path);
+#else
+ PathService::Get(base::DIR_MODULE, &pak_path);
+#endif
+
+ base::FilePath ui_test_pak_path;
+ ASSERT_TRUE(PathService::Get(ui::UI_TEST_PAK, &ui_test_pak_path));
+ ui::ResourceBundle::InitSharedInstanceWithPakPath(ui_test_pak_path);
+
+ ui::ResourceBundle::GetSharedInstance().AddDataPackFromPath(
+ pak_path.AppendASCII("components_tests_resources.pak"),
+ ui::SCALE_FACTOR_NONE);
}

void PreloadIcon(const GURL& url,
@@ -67,22 +140,29 @@
}

bool IconIsCachedFor(const GURL& url, favicon_base::IconType icon_type) {
+ return !GetCachedIconFor(url, icon_type).IsEmpty();
+ }
+
+ gfx::Image GetCachedIconFor(const GURL& url,
+ favicon_base::IconType icon_type) {
base::CancelableTaskTracker tracker;
- bool is_cached;
+ gfx::Image image;
base::RunLoop loop;
favicon::GetFaviconImageForPageURL(
&favicon_service_, url, icon_type,
base::Bind(
- [](bool* is_cached, base::RunLoop* loop,
+ [](gfx::Image* image, base::RunLoop* loop,
const favicon_base::FaviconImageResult& result) {
- *is_cached = !result.image.IsEmpty();
+ *image = result.image;
loop->Quit();
},
- &is_cached, &loop),
+ &image, &loop),
&tracker);
loop.Run();
- return is_cached;
+ return image;
}
+
+ void WaitForTasksToFinish() { task_runner_->RunUntilIdle(); }

base::MessageLoop message_loop_;
PopularSites::Site site_;
@@ -90,12 +170,9 @@
base::ScopedTempDir history_dir_;
history::HistoryService history_service_;
favicon::FaviconServiceImpl favicon_service_;
+ scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
+ NiceMock<MockResourceDelegate> mock_resource_delegate_;
};
-
-template <typename Fn>
-base::Callback<Fn> BindMockFunction(MockFunction<Fn>* function) {
- return base::Bind(&MockFunction<Fn>::Call, base::Unretained(function));
-}

ACTION(FailFetch) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ -112,7 +189,8 @@
}

TEST_F(IconCacherTest, LargeCached) {
- MockFunction<void(bool)> done;
+ base::MockCallback<base::Closure> done;
+ EXPECT_CALL(done, Run()).Times(0);
base::RunLoop loop;
{
InSequence s;
@@ -120,20 +198,18 @@
SetDataUseServiceName(
data_use_measurement::DataUseUserData::NTP_TILES));
EXPECT_CALL(*image_fetcher_, SetDesiredImageFrameSize(gfx::Size(128, 128)));
- EXPECT_CALL(done, Call(false)).WillOnce(Quit(&loop));
}
PreloadIcon(site_.url, site_.large_icon_url, favicon_base::TOUCH_ICON, 128,
128);
-
IconCacherImpl cacher(&favicon_service_, std::move(image_fetcher_));
- cacher.StartFetch(site_, BindMockFunction(&done));
- loop.Run();
+ cacher.StartFetch(site_, done.Get(), done.Get());
+ WaitForTasksToFinish();
EXPECT_FALSE(IconIsCachedFor(site_.url, favicon_base::FAVICON));
EXPECT_TRUE(IconIsCachedFor(site_.url, favicon_base::TOUCH_ICON));
}

TEST_F(IconCacherTest, LargeNotCachedAndFetchSucceeded) {
- MockFunction<void(bool)> done;
+ base::MockCallback<base::Closure> done;
base::RunLoop loop;
{
InSequence s;
@@ -144,11 +220,11 @@
EXPECT_CALL(*image_fetcher_,
StartOrQueueNetworkRequest(_, site_.large_icon_url, _))
.WillOnce(PassFetch(128, 128));
- EXPECT_CALL(done, Call(true)).WillOnce(Quit(&loop));
+ EXPECT_CALL(done, Run()).WillOnce(Quit(&loop));
}

IconCacherImpl cacher(&favicon_service_, std::move(image_fetcher_));
- cacher.StartFetch(site_, BindMockFunction(&done));
+ cacher.StartFetch(site_, done.Get(), done.Get());
loop.Run();
EXPECT_FALSE(IconIsCachedFor(site_.url, favicon_base::FAVICON));
EXPECT_TRUE(IconIsCachedFor(site_.url, favicon_base::TOUCH_ICON));
@@ -157,7 +233,7 @@
TEST_F(IconCacherTest, SmallNotCachedAndFetchSucceeded) {
site_.large_icon_url = GURL();

- MockFunction<void(bool)> done;
+ base::MockCallback<base::Closure> done;
base::RunLoop loop;
{
InSequence s;
@@ -168,19 +244,19 @@
EXPECT_CALL(*image_fetcher_,
StartOrQueueNetworkRequest(_, site_.favicon_url, _))
.WillOnce(PassFetch(128, 128));
- EXPECT_CALL(done, Call(true)).WillOnce(Quit(&loop));
+ EXPECT_CALL(done, Run()).WillOnce(Quit(&loop));
}

IconCacherImpl cacher(&favicon_service_, std::move(image_fetcher_));
- cacher.StartFetch(site_, BindMockFunction(&done));
+ cacher.StartFetch(site_, done.Get(), done.Get());
loop.Run();
EXPECT_TRUE(IconIsCachedFor(site_.url, favicon_base::FAVICON));
EXPECT_FALSE(IconIsCachedFor(site_.url, favicon_base::TOUCH_ICON));
}

TEST_F(IconCacherTest, LargeNotCachedAndFetchFailed) {
- MockFunction<void(bool)> done;
- base::RunLoop loop;
+ base::MockCallback<base::Closure> done;
+ EXPECT_CALL(done, Run()).Times(0);
{
InSequence s;
EXPECT_CALL(*image_fetcher_,
@@ -190,15 +266,54 @@
EXPECT_CALL(*image_fetcher_,
StartOrQueueNetworkRequest(_, site_.large_icon_url, _))
.WillOnce(FailFetch());
- EXPECT_CALL(done, Call(false)).WillOnce(Quit(&loop));
}

IconCacherImpl cacher(&favicon_service_, std::move(image_fetcher_));
- cacher.StartFetch(site_, BindMockFunction(&done));
- loop.Run();
+ cacher.StartFetch(site_, done.Get(), done.Get());
+ WaitForTasksToFinish();
EXPECT_FALSE(IconIsCachedFor(site_.url, favicon_base::FAVICON));
EXPECT_FALSE(IconIsCachedFor(site_.url, favicon_base::TOUCH_ICON));
}

+TEST_F(IconCacherTest, ProvidesDefaultIconAndSucceedsWithFetching) {
+ // We are not interested which delegate function actually handles the call to
+ // |GetNativeImageNamed| as long as we receive the right image.
+ ON_CALL(mock_resource_delegate_, GetNativeImageNamed(12345))
+ .WillByDefault(Return(gfx::test::CreateImage(64, 64)));
+ ON_CALL(mock_resource_delegate_, GetImageNamed(12345))
+ .WillByDefault(Return(gfx::test::CreateImage(64, 64)));
+ base::MockCallback<base::Closure> preliminary_icon_available;
+ base::MockCallback<base::Closure> icon_available;
+ base::RunLoop default_loop;
+ base::RunLoop fetch_loop;
+ {
+ InSequence s;
+ EXPECT_CALL(*image_fetcher_,
+ SetDataUseServiceName(
+ data_use_measurement::DataUseUserData::NTP_TILES));
+ EXPECT_CALL(*image_fetcher_, SetDesiredImageFrameSize(gfx::Size(128, 128)));
+ EXPECT_CALL(preliminary_icon_available, Run())
+ .WillOnce(Quit(&default_loop));
+ EXPECT_CALL(*image_fetcher_,
+ StartOrQueueNetworkRequest(_, site_.large_icon_url, _))
+ .WillOnce(PassFetch(128, 128));
+ EXPECT_CALL(icon_available, Run()).WillOnce(Quit(&fetch_loop));
+ }
+
+ IconCacherImpl cacher(&favicon_service_, std::move(image_fetcher_));
+ site_.default_icon_resource = 12345;
+ cacher.StartFetch(site_, icon_available.Get(),
+ preliminary_icon_available.Get());
+
+ default_loop.Run(); // Wait for the default image.
+ EXPECT_THAT(GetCachedIconFor(site_.url, favicon_base::TOUCH_ICON).Size(),
+ Eq(gfx::Size(64, 64))); // Compares dimensions, not objects.
+
+ // Let the fetcher continue and wait for the second call of the callback.
+ fetch_loop.Run(); // Wait for the updated image.
+ EXPECT_THAT(GetCachedIconFor(site_.url, favicon_base::TOUCH_ICON).Size(),
+ Eq(gfx::Size(128, 128))); // Compares dimensions, not objects.
+}
+
} // namespace
} // namespace ntp_tiles
diff --git a/components/ntp_tiles/most_visited_sites.cc b/components/ntp_tiles/most_visited_sites.cc
index f3695c9..4f7c845 100644
--- a/components/ntp_tiles/most_visited_sites.cc
+++ b/components/ntp_tiles/most_visited_sites.cc
@@ -359,9 +359,10 @@
tile.source = NTPTileSource::POPULAR;

popular_sites_tiles.push_back(std::move(tile));
- icon_cacher_->StartFetch(
- popular_site, base::Bind(&MostVisitedSites::OnIconMadeAvailable,
- base::Unretained(this), popular_site.url));
+ base::Closure icon_available =
+ base::Bind(&MostVisitedSites::OnIconMadeAvailable,
+ base::Unretained(this), popular_site.url);
+ icon_cacher_->StartFetch(popular_site, icon_available, icon_available);
if (popular_sites_tiles.size() >= num_popular_sites_tiles)
break;
}
@@ -420,10 +421,8 @@
}
}

-void MostVisitedSites::OnIconMadeAvailable(const GURL& site_url,
- bool newly_available) {
- if (newly_available)
- observer_->OnIconMadeAvailable(site_url);
+void MostVisitedSites::OnIconMadeAvailable(const GURL& site_url) {
+ observer_->OnIconMadeAvailable(site_url);
}

void MostVisitedSites::TopSitesLoaded(TopSites* top_sites) {}
diff --git a/components/ntp_tiles/most_visited_sites.h b/components/ntp_tiles/most_visited_sites.h
index 5df1fc66..12cf44e 100644
--- a/components/ntp_tiles/most_visited_sites.h
+++ b/components/ntp_tiles/most_visited_sites.h
@@ -181,7 +181,7 @@

void OnPopularSitesDownloaded(bool success);

- void OnIconMadeAvailable(const GURL& site_url, bool newly_available);
+ void OnIconMadeAvailable(const GURL& site_url);

// history::TopSitesObserver implementation.
void TopSitesLoaded(history::TopSites* top_sites) override;
diff --git a/components/ntp_tiles/most_visited_sites_unittest.cc b/components/ntp_tiles/most_visited_sites_unittest.cc
index 0142bad..0a7a5b5 100644
--- a/components/ntp_tiles/most_visited_sites_unittest.cc
+++ b/components/ntp_tiles/most_visited_sites_unittest.cc
@@ -193,9 +193,10 @@

class MockIconCacher : public IconCacher {
public:
- MOCK_METHOD2(StartFetch,
+ MOCK_METHOD3(StartFetch,
void(PopularSites::Site site,
- const base::Callback<void(bool)>& done));
+ const base::Closure& icon_available,
+ const base::Closure& preliminary_icon_available));
};

class PopularSitesFactoryForTest {
@@ -311,7 +312,7 @@
.WillRepeatedly(Return(false));
// Mock icon cacher never replies, and we also don't verify whether the
// code uses it correctly.
- EXPECT_CALL(*icon_cacher, StartFetch(_, _)).Times(AtLeast(0));
+ EXPECT_CALL(*icon_cacher, StartFetch(_, _, _)).Times(AtLeast(0));
}

most_visited_sites_ = base::MakeUnique<MostVisitedSites>(
diff --git a/components/ntp_tiles/popular_sites.h b/components/ntp_tiles/popular_sites.h
index dde8459..43dbc84 100644
--- a/components/ntp_tiles/popular_sites.h
+++ b/components/ntp_tiles/popular_sites.h
@@ -37,6 +37,7 @@
GURL favicon_url;
GURL large_icon_url;
GURL thumbnail_url;
+ int default_icon_resource; // < 0 if there is none. Used for popular sites.
};

using SitesVector = std::vector<Site>;
diff --git a/components/ntp_tiles/popular_sites_impl.cc b/components/ntp_tiles/popular_sites_impl.cc
index 6e443ab..7777e37 100644
--- a/components/ntp_tiles/popular_sites_impl.cc
+++ b/components/ntp_tiles/popular_sites_impl.cc
@@ -130,21 +130,46 @@

sites.emplace_back(title, GURL(url), GURL(favicon_url),
GURL(large_icon_url), GURL(thumbnail_url));
+ item->GetInteger("default_icon_resource",
+ &sites.back().default_icon_resource);
}
return sites;
}

+#if defined(GOOGLE_CHROME_BUILD) && (defined(OS_ANDROID) || defined(OS_IOS))
+void SetDefaultResourceForSite(int index,
+ int resource_id,
+ base::ListValue* sites) {
+ base::DictionaryValue* site;
+ if (!sites->GetDictionary(index, &site)) {
+ return;
+ }
+ site->SetInteger("default_icon_resource", resource_id);
+}
+#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)
+#if !defined(OS_ANDROID) && !defined(OS_IOS)
+ return base::MakeUnique<base::ListValue>();
+#else
std::unique_ptr<base::ListValue> sites =
- base::ListValue::From(base::JSONReader().ReadToValue(
+ base::ListValue::From(base::JSONReader::Read(
ResourceBundle::GetSharedInstance().GetRawDataResource(
IDR_DEFAULT_POPULAR_SITES_JSON)));
DCHECK(sites);
+#if defined(GOOGLE_CHROME_BUILD)
+ int index = 0;
+ for (int icon_resource :
+ {IDR_DEFAULT_POPULAR_SITES_ICON0, IDR_DEFAULT_POPULAR_SITES_ICON1,
+ IDR_DEFAULT_POPULAR_SITES_ICON2, IDR_DEFAULT_POPULAR_SITES_ICON3,
+ IDR_DEFAULT_POPULAR_SITES_ICON4, IDR_DEFAULT_POPULAR_SITES_ICON5,
+ IDR_DEFAULT_POPULAR_SITES_ICON6, IDR_DEFAULT_POPULAR_SITES_ICON7}) {
+ SetDefaultResourceForSite(index++, icon_resource, sites.get());
+ }
+#endif // GOOGLE_CHROME_BUILD
return sites;
-#endif
- return base::MakeUnique<base::ListValue>();
+#endif // OS_ANDROID || OS_IOS
}

} // namespace
@@ -158,7 +183,8 @@
url(url),
favicon_url(favicon_url),
large_icon_url(large_icon_url),
- thumbnail_url(thumbnail_url) {}
+ thumbnail_url(thumbnail_url),
+ default_icon_resource(-1) {}

PopularSites::Site::Site(const Site& other) = default;

diff --git a/components/ntp_tiles/popular_sites_impl_unittest.cc b/components/ntp_tiles/popular_sites_impl_unittest.cc
index 6732e27..a80aa92 100644
--- a/components/ntp_tiles/popular_sites_impl_unittest.cc
+++ b/components/ntp_tiles/popular_sites_impl_unittest.cc
@@ -34,6 +34,7 @@
#include "testing/gtest/include/gtest/gtest.h"

using testing::Eq;
+using testing::Gt;
using testing::IsEmpty;

namespace ntp_tiles {
@@ -169,7 +170,17 @@
net::FakeURLFetcherFactory url_fetcher_factory_;
};

-TEST_F(PopularSitesTest, Basic) {
+TEST_F(PopularSitesTest, ContainsDefaultTilesRightAfterConstruction) {
+ scoped_refptr<net::TestURLRequestContextGetter> url_request_context(
+ new net::TestURLRequestContextGetter(
+ base::ThreadTaskRunnerHandle::Get()));
+
+ auto popular_sites = CreatePopularSites(url_request_context.get());
+ EXPECT_THAT(popular_sites->sites().size(),
+ Eq(GetNumberOfDefaultPopularSitesForPlatform()));
+}
+
+TEST_F(PopularSitesTest, Zasic) {
SetCountryAndVersion("ZZ", "9");
RespondWithJSON(
"https://www.gstatic.com/chrome/ntp/suggested_sites_ZZ_9.json",
@@ -185,15 +196,6 @@
EXPECT_THAT(sites[0].large_icon_url,
URLEq("https://zz.m.wikipedia.org/wikipedia.png"));
EXPECT_THAT(sites[0].favicon_url, URLEq(""));
-}
-
-TEST_F(PopularSitesTest, ContainsDefaultTilesRightAfterConstruction) {
- scoped_refptr<net::TestURLRequestContextGetter> url_request_context(
- new net::TestURLRequestContextGetter(
- base::ThreadTaskRunnerHandle::Get()));
-
- EXPECT_THAT(CreatePopularSites(url_request_context.get())->sites().size(),
- Eq(GetNumberOfDefaultPopularSitesForPlatform()));
}

TEST_F(PopularSitesTest, Fallback) {
@@ -234,6 +236,21 @@
EXPECT_THAT(sites.size(), Eq(GetNumberOfDefaultPopularSitesForPlatform()));
}

+TEST_F(PopularSitesTest, AddsIconResourcesToDefaultPages) {
+ scoped_refptr<net::TestURLRequestContextGetter> url_request_context(
+ new net::TestURLRequestContextGetter(
+ base::ThreadTaskRunnerHandle::Get()));
+ std::unique_ptr<PopularSites> popular_sites =
+ CreatePopularSites(url_request_context.get());
+
+#if defined(GOOGLE_CHROME_BUILD)
+ ASSERT_FALSE(popular_sites->sites().empty());
+ for (const auto& site : popular_sites->sites()) {
+ EXPECT_THAT(site.default_icon_resource, Gt(0));
+ }
+#endif
+}
+
TEST_F(PopularSitesTest, ProvidesDefaultSitesUntilCallbackReturns) {
SetCountryAndVersion("ZZ", "9");
RespondWithJSON(
diff --git a/components/ntp_tiles/resources/default_popular_sites.json b/components/ntp_tiles/resources/default_popular_sites.json
index 9e14a78..a41a914 100644
--- a/components/ntp_tiles/resources/default_popular_sites.json
+++ b/components/ntp_tiles/resources/default_popular_sites.json
@@ -1,42 +1,42 @@
[
- {
- "title": "Facebook - Log In or Sign Up",
- "url": "https://m.facebook.com/",
- "large_icon_url": "https://static.xx.fbcdn.net/rsrc.php/v3/ya/r/O2aKM2iSbOw.png"
- },
- {
- "title": "YouTube",
- "url": "https://m.youtube.com/",
- "large_icon_url": "https://s.ytimg.com/yts/mobile/img/apple-touch-icon-144x144-precomposed-vflwq-hLZ.png"
- },
- {
- "title": "Amazon.com: Online Shopping for Electronics, Apparel, Computers, Books, DVDs \u0026 more",
- "url": "https://www.amazon.com/",
- "large_icon_url": "https://images-na.ssl-images-amazon.com/images/G/01/anywhere/a_smile_196x196._CB368246573_.png"
- },
- {
- "title": "Wikipedia, the free encyclopedia",
- "url": "https://en.m.wikipedia.org/",
- "large_icon_url": "https://en.m.wikipedia.org/static/apple-touch/wikipedia.png"
- },
- {
- "title": "ESPN: The Worldwide Leader in Sports",
- "url": "http://www.espn.com/",
- "large_icon_url": "http://a.espncdn.com/wireless/mw5/r1/images/bookmark-icons/espn_icon-152x152.min.png"
- },
- {
- "title": "Yahoo",
- "url": "https://www.yahoo.com/",
- "large_icon_url": "https://s.yimg.com/dh/ap/default/130909/y_200_a.png"
- },
- {
- "title": "Instagram",
- "url": "https://www.instagram.com/",
- "large_icon_url": "https://instagramstatic-a.akamaihd.net/h1/images/ico/favicon-192.png/b407fa101800.png"
- },
- {
- "title": "Electronics, Cars, Fashion, Collectibles, Coupons and More | eBay",
- "url": "http://m.ebay.com/",
- "large_icon_url": "http://ir.ebaystatic.com/pictures/aw/pics/mobile/images/apple-touch-icon.png"
- }
+ {
+ "large_icon_url": "https://www.chromium.org/_/rsrc/1484148948583/apple-touch-icon.png",
+ "title": "The Chromium Projects `",
+ "url": "https://www.chromium.org/"
+ },
+ {
+ "favicon_url": "https://codereview.chromium.org/static/favicon.ico",
+ "title": "Chromium Code Reviews",
+ "url": "https://codereview.chromium.org/"
+ },
+ {
+ "large_icon_url": "https://www.chromium.org/_/rsrc/1484148948583/apple-touch-icon.png",
+ "title": "Git Repositories on chromium",
+ "url": "https://chromium.googlesource.com/"
+ },
+ {
+ "large_icon_url": "https://developers.google.com/_static/9bce7b6017/images/touch-icon.png",
+ "title": "Google Open Soruce Programs Office",
+ "url": "https://developers.google.com/open-source/"
+ },
+ {
+ "large_icon_url": "https://freenode.net/static/img/logos/coloured-alphabg-sq-120.png",
+ "title": "freenode",
+ "url": "https://freenode.net/"
+ },
+ {
+ "large_icon_url": "https://assets-cdn.github.com/pinned-octocat.svg",
+ "title": "Github Page of the Chromium Project",
+ "url": "https://github.com/chromium/"
+ },
+ {
+ "large_icon_url": "https://cdn2.iconfinder.com/data/icons/minimalism/512/Chromium.png",
+ "title": "Chrome Flags",
+ "url": "chrome://flags"
+ },
+ {
+ "large_icon_url": "https://cdn2.iconfinder.com/data/icons/minimalism/512/Chromium.png",
+ "title": "Chrome Version",
+ "url": "chrome://version/"
+ }
]
diff --git a/components/resources/ntp_tiles_resources.grdp b/components/resources/ntp_tiles_resources.grdp
index e42d4ea..750d7d1 100644
--- a/components/resources/ntp_tiles_resources.grdp
+++ b/components/resources/ntp_tiles_resources.grdp
@@ -4,7 +4,23 @@
<include name="IDR_POPULAR_SITES_INTERNALS_HTML" file="../ntp_tiles/webui/resources/popular_sites_internals.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" />
<include name="IDR_POPULAR_SITES_INTERNALS_JS" file="../ntp_tiles/webui/resources/popular_sites_internals.js" type="BINDATA" />
<include name="IDR_POPULAR_SITES_INTERNALS_CSS" file="../ntp_tiles/webui/resources/popular_sites_internals.css" type="BINDATA" />
- <include name="IDR_DEFAULT_POPULAR_SITES_JSON" file="../ntp_tiles/resources/default_popular_sites.json" type="BINDATA" />
+ <if expr="_google_chrome">
+ <then>
+ <include name="IDR_DEFAULT_POPULAR_SITES_JSON" file="../ntp_tiles/resources/internal/default_popular_sites.json" type="BINDATA" />
+ <include name="IDR_DEFAULT_POPULAR_SITES_ICON0" file="../ntp_tiles/resources/internal/icon0.png" type="BINDATA" />
+ <include name="IDR_DEFAULT_POPULAR_SITES_ICON1" file="../ntp_tiles/resources/internal/icon1.png" type="BINDATA" />
+ <include name="IDR_DEFAULT_POPULAR_SITES_ICON2" file="../ntp_tiles/resources/internal/icon2.png" type="BINDATA" />
+ <include name="IDR_DEFAULT_POPULAR_SITES_ICON3" file="../ntp_tiles/resources/internal/icon3.png" type="BINDATA" />
+ <include name="IDR_DEFAULT_POPULAR_SITES_ICON4" file="../ntp_tiles/resources/internal/icon4.png" type="BINDATA" />
+ <include name="IDR_DEFAULT_POPULAR_SITES_ICON5" file="../ntp_tiles/resources/internal/icon5.png" type="BINDATA" />
+ <include name="IDR_DEFAULT_POPULAR_SITES_ICON6" file="../ntp_tiles/resources/internal/icon6.png" type="BINDATA" />
+ <include name="IDR_DEFAULT_POPULAR_SITES_ICON7" file="../ntp_tiles/resources/internal/icon7.png" type="BINDATA" />
+ </then>
+ <else>
+ <!-- Fall back to a local resource so popular sites can be tested properly. -->
+ <include name="IDR_DEFAULT_POPULAR_SITES_JSON" file="../ntp_tiles/resources/default_popular_sites.json" type="BINDATA" />
+ </else>
+ </if>
</if>
<include name="IDR_NTP_TILES_INTERNALS_HTML" file="../ntp_tiles/webui/resources/ntp_tiles_internals.html" flattenhtml="true" allowexternalscript="true" type="BINDATA" />
<include name="IDR_NTP_TILES_INTERNALS_JS" file="../ntp_tiles/webui/resources/ntp_tiles_internals.js" type="BINDATA" />