diff options
Diffstat (limited to 'chromium/components/favicon')
9 files changed, 203 insertions, 148 deletions
diff --git a/chromium/components/favicon/content/content_favicon_driver.cc b/chromium/components/favicon/content/content_favicon_driver.cc index 355da7b8faa..ead4610292a 100644 --- a/chromium/components/favicon/content/content_favicon_driver.cc +++ b/chromium/components/favicon/content/content_favicon_driver.cc @@ -19,21 +19,6 @@ #include "ui/gfx/image/image.h" namespace favicon { -namespace { - -void ExtractManifestIcons( - ContentFaviconDriver::ManifestDownloadCallback callback, - const GURL& manifest_url, - const blink::Manifest& manifest) { - std::vector<FaviconURL> candidates; - for (const auto& icon : manifest.icons) { - candidates.emplace_back(icon.src, favicon_base::IconType::kWebManifestIcon, - icon.sizes); - } - std::move(callback).Run(candidates); -} - -} // namespace // static void ContentFaviconDriver::CreateForWebContents( @@ -73,11 +58,8 @@ gfx::Image ContentFaviconDriver::GetFavicon() const { // Like GetTitle(), we also want to use the favicon for the last committed // entry rather than a pending navigation entry. content::NavigationController& controller = web_contents()->GetController(); - content::NavigationEntry* entry = controller.GetTransientEntry(); - if (entry) - return entry->GetFavicon().image; - entry = controller.GetLastCommittedEntry(); + content::NavigationEntry* entry = controller.GetLastCommittedEntry(); if (entry) return entry->GetFavicon().image; return gfx::Image(); @@ -85,11 +67,8 @@ gfx::Image ContentFaviconDriver::GetFavicon() const { bool ContentFaviconDriver::FaviconIsValid() const { content::NavigationController& controller = web_contents()->GetController(); - content::NavigationEntry* entry = controller.GetTransientEntry(); - if (entry) - return entry->GetFavicon().valid; - entry = controller.GetLastCommittedEntry(); + content::NavigationEntry* entry = controller.GetLastCommittedEntry(); if (entry) return entry->GetFavicon().valid; @@ -108,7 +87,25 @@ ContentFaviconDriver::ContentFaviconDriver(content::WebContents* web_contents, FaviconDriverImpl(favicon_service), document_on_load_completed_(false) {} -ContentFaviconDriver::~ContentFaviconDriver() { +ContentFaviconDriver::~ContentFaviconDriver() = default; + +void ContentFaviconDriver::OnDidDownloadManifest( + ManifestDownloadCallback callback, + const GURL& manifest_url, + const blink::Manifest& manifest) { + // ~WebContentsImpl triggers running any pending callbacks for manifests. + // As we're about to be destroyed ignore the request. To do otherwise may + // result in calling back to this and attempting to use the WebContents, which + // will crash. + if (!web_contents()) + return; + + std::vector<FaviconURL> candidates; + for (const auto& icon : manifest.icons) { + candidates.emplace_back(icon.src, favicon_base::IconType::kWebManifestIcon, + icon.sizes); + } + std::move(callback).Run(candidates); } int ContentFaviconDriver::DownloadImage(const GURL& url, @@ -125,7 +122,8 @@ int ContentFaviconDriver::DownloadImage(const GURL& url, void ContentFaviconDriver::DownloadManifest(const GURL& url, ManifestDownloadCallback callback) { web_contents()->GetManifest( - base::BindOnce(&ExtractManifestIcons, std::move(callback))); + base::BindOnce(&ContentFaviconDriver::OnDidDownloadManifest, + base::Unretained(this), std::move(callback))); } bool ContentFaviconDriver::IsOffTheRecord() { @@ -173,6 +171,7 @@ void ContentFaviconDriver::OnFaviconDeleted( } void ContentFaviconDriver::DidUpdateFaviconURL( + content::RenderFrameHost* rfh, const std::vector<blink::mojom::FaviconURLPtr>& candidates) { // Ignore the update if there is no last committed navigation entry. This can // occur when loading an initially blank page. @@ -198,6 +197,7 @@ void ContentFaviconDriver::DidUpdateFaviconURL( } void ContentFaviconDriver::DidUpdateWebManifestURL( + content::RenderFrameHost* rfh, const base::Optional<GURL>& manifest_url) { // Ignore the update if there is no last committed navigation entry. This can // occur when loading an initially blank page. diff --git a/chromium/components/favicon/content/content_favicon_driver.h b/chromium/components/favicon/content/content_favicon_driver.h index 24bcac5636c..3bc3b904a42 100644 --- a/chromium/components/favicon/content/content_favicon_driver.h +++ b/chromium/components/favicon/content/content_favicon_driver.h @@ -53,6 +53,11 @@ class ContentFaviconDriver private: friend class content::WebContentsUserData<ContentFaviconDriver>; + // Callback when a manifest is downloaded. + void OnDidDownloadManifest(ManifestDownloadCallback callback, + const GURL& manifest_url, + const blink::Manifest& manifest); + // FaviconHandler::Delegate implementation. int DownloadImage(const GURL& url, int max_image_size, @@ -71,8 +76,10 @@ class ContentFaviconDriver // content::WebContentsObserver implementation. void DidUpdateFaviconURL( + content::RenderFrameHost* rfh, const std::vector<blink::mojom::FaviconURLPtr>& candidates) override; void DidUpdateWebManifestURL( + content::RenderFrameHost* rfh, const base::Optional<GURL>& manifest_url) override; void DidStartNavigation( content::NavigationHandle* navigation_handle) override; diff --git a/chromium/components/favicon/content/content_favicon_driver_unittest.cc b/chromium/components/favicon/content/content_favicon_driver_unittest.cc index eb11ffbffc8..d8ded21da4b 100644 --- a/chromium/components/favicon/content/content_favicon_driver_unittest.cc +++ b/chromium/components/favicon/content/content_favicon_driver_unittest.cc @@ -9,9 +9,11 @@ #include "base/macros.h" #include "base/run_loop.h" +#include "base/test/bind_test_util.h" #include "base/test/metrics/histogram_tester.h" #include "components/favicon/core/favicon_client.h" #include "components/favicon/core/favicon_handler.h" +#include "components/favicon/core/test/favicon_driver_impl_test_helper.h" #include "components/favicon/core/test/mock_favicon_service.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/test/navigation_simulator.h" @@ -38,15 +40,16 @@ class ContentFaviconDriverTest : public content::RenderViewHostTestHarness { const GURL kIconURL = GURL("http://www.google.com/favicon.ico"); ContentFaviconDriverTest() { + ContentFaviconDriverTest* t = this; ON_CALL(favicon_service_, UpdateFaviconMappingsAndFetch(_, _, _, _, _, _)) - .WillByDefault([](auto, auto, auto, auto, - favicon_base::FaviconResultsCallback callback, - base::CancelableTaskTracker* tracker) { + .WillByDefault([t](auto, auto, auto, auto, + favicon_base::FaviconResultsCallback callback, + base::CancelableTaskTracker* tracker) { return tracker->PostTask( base::ThreadTaskRunnerHandle::Get().get(), FROM_HERE, - base::BindOnce( - std::move(callback), - std::vector<favicon_base::FaviconRawBitmapResult>())); + base::BindOnce(&ContentFaviconDriverTest:: + OnCallUpdateFaviconMappingsAndFetch, + base::Unretained(t), std::move(callback))); }); ON_CALL(favicon_service_, GetFaviconForPageURL(_, _, _, _, _)) .WillByDefault([](auto, auto, auto, @@ -60,7 +63,7 @@ class ContentFaviconDriverTest : public content::RenderViewHostTestHarness { }); } - ~ContentFaviconDriverTest() override {} + ~ContentFaviconDriverTest() override = default; // content::RenderViewHostTestHarness: void SetUp() override { @@ -81,11 +84,25 @@ class ContentFaviconDriverTest : public content::RenderViewHostTestHarness { ContentFaviconDriver::FromWebContents(web_contents()); web_contents_tester()->NavigateAndCommit(page_url); static_cast<content::WebContentsObserver*>(favicon_driver) - ->DidUpdateFaviconURL(candidates); + ->DidUpdateFaviconURL(web_contents()->GetMainFrame(), candidates); base::RunLoop().RunUntilIdle(); } + // This is run *after* the callback posted by way of + // UpdateFaviconMappingsAndFetch() is run. It allows hooking processing + // immediately after it. + base::OnceClosure on_did_update_favicon_mappings_and_fetch_; + testing::NiceMock<MockFaviconService> favicon_service_; + + private: + // Callback from UpdateFaviconMappingsAndFetch(). + void OnCallUpdateFaviconMappingsAndFetch( + favicon_base::FaviconResultsCallback callback) { + std::move(callback).Run({}); + if (on_did_update_favicon_mappings_and_fetch_) + std::move(on_did_update_favicon_mappings_and_fetch_).Run(); + } }; // Test that a download is initiated when there isn't a favicon in the database @@ -113,7 +130,7 @@ TEST_F(ContentFaviconDriverTest, ShouldNotCauseImageDownload) { favicon_urls.push_back(blink::mojom::FaviconURL::New( kIconURL, blink::mojom::FaviconIconType::kFavicon, kEmptyIconSizes)); static_cast<content::WebContentsObserver*>(favicon_driver) - ->DidUpdateFaviconURL(favicon_urls); + ->DidUpdateFaviconURL(web_contents()->GetMainFrame(), favicon_urls); base::RunLoop().RunUntilIdle(); EXPECT_FALSE(web_contents_tester()->HasPendingDownloadImage(kIconURL)); @@ -164,12 +181,46 @@ TEST_F(ContentFaviconDriverTest, FaviconUpdateNoLastCommittedEntry) { blink::mojom::FaviconIconType::kFavicon, kEmptyIconSizes)); favicon::ContentFaviconDriver* driver = favicon::ContentFaviconDriver::FromWebContents(web_contents()); - static_cast<content::WebContentsObserver*>(driver) - ->DidUpdateFaviconURL(favicon_urls); + static_cast<content::WebContentsObserver*>(driver)->DidUpdateFaviconURL( + web_contents()->GetMainFrame(), favicon_urls); // Test that ContentFaviconDriver ignored the favicon url update. EXPECT_TRUE(driver->favicon_urls().empty()); } +// This test verifies a crash doesn't happen during deletion of the +// WebContents. The crash occurred because ~WebContentsImpl would trigger +// running callbacks for manifests. This mean FaviconHandler would be called +// while ContentFaviconDriver::web_contents() was null, which is unexpected and +// crashed. See https://crbug.com/1114237 for more. +TEST_F(ContentFaviconDriverTest, + WebContentsDeletedWithInProgressManifestRequest) { + // Manifests are only downloaded with TOUCH_LARGEST. Force creating this + // handler so code path is exercised on all platforms. + favicon::ContentFaviconDriver* driver = + favicon::ContentFaviconDriver::FromWebContents(web_contents()); + FaviconDriverImplTestHelper::RecreateHandlerForType( + driver, FaviconDriverObserver::TOUCH_LARGEST); + + // Mimic a page load. + std::vector<blink::mojom::FaviconURLPtr> favicon_urls; + favicon_urls.push_back(blink::mojom::FaviconURL::New( + kIconURL, blink::mojom::FaviconIconType::kTouchIcon, kEmptyIconSizes)); + TestFetchFaviconForPage(kPageURL, favicon_urls); + + // Trigger downloading a manifest. + base::RunLoop run_loop; + on_did_update_favicon_mappings_and_fetch_ = + base::BindLambdaForTesting([&] { run_loop.Quit(); }); + static_cast<content::WebContentsObserver*>(driver)->DidUpdateWebManifestURL( + web_contents()->GetMainFrame(), GURL("http://bad.manifest.com")); + run_loop.Run(); + + // The request for the manifest is still pending, delete the WebContents, + // which should trigger notifying the callback for the manifest and *not* + // crash. + DeleteContents(); +} + } // namespace } // namespace favicon diff --git a/chromium/components/favicon/core/favicon_driver_impl.cc b/chromium/components/favicon/core/favicon_driver_impl.cc index 3817b3f5f5d..916ded30997 100644 --- a/chromium/components/favicon/core/favicon_driver_impl.cc +++ b/chromium/components/favicon/core/favicon_driver_impl.cc @@ -6,7 +6,6 @@ #include <memory> -#include "base/metrics/histogram_macros.h" #include "base/strings/string_util.h" #include "build/build_config.h" #include "components/favicon/core/favicon_driver_observer.h" @@ -41,8 +40,7 @@ FaviconDriverImpl::FaviconDriverImpl(FaviconService* favicon_service) } } -FaviconDriverImpl::~FaviconDriverImpl() { -} +FaviconDriverImpl::~FaviconDriverImpl() = default; void FaviconDriverImpl::FetchFavicon(const GURL& page_url, bool is_same_document) { diff --git a/chromium/components/favicon/core/favicon_driver_impl.h b/chromium/components/favicon/core/favicon_driver_impl.h index a7bf33d660e..1251b29b681 100644 --- a/chromium/components/favicon/core/favicon_driver_impl.h +++ b/chromium/components/favicon/core/favicon_driver_impl.h @@ -54,6 +54,8 @@ class FaviconDriverImpl : public FaviconDriver, FaviconService* favicon_service() { return favicon_service_; } private: + friend class FaviconDriverImplTestHelper; + // KeyedService used by FaviconDriverImpl. It may be null during testing, // but if it is defined, it must outlive the FaviconDriverImpl. FaviconService* favicon_service_; diff --git a/chromium/components/favicon/core/favicon_handler.cc b/chromium/components/favicon/core/favicon_handler.cc index 95a979c8521..64eab051de7 100644 --- a/chromium/components/favicon/core/favicon_handler.cc +++ b/chromium/components/favicon/core/favicon_handler.cc @@ -12,6 +12,7 @@ #include "base/bind.h" #include "base/bind_helpers.h" #include "base/feature_list.h" +#include "base/logging.h" #include "base/memory/ref_counted_memory.h" #include "base/metrics/histogram_functions.h" #include "base/metrics/histogram_macros.h" diff --git a/chromium/components/favicon/core/history_ui_favicon_request_handler_impl_unittest.cc b/chromium/components/favicon/core/history_ui_favicon_request_handler_impl_unittest.cc index ef6a2081996..dee24c1680e 100644 --- a/chromium/components/favicon/core/history_ui_favicon_request_handler_impl_unittest.cc +++ b/chromium/components/favicon/core/history_ui_favicon_request_handler_impl_unittest.cc @@ -7,6 +7,7 @@ #include <utility> #include "base/bind.h" +#include "base/bind_helpers.h" #include "base/task/cancelable_task_tracker.h" #include "base/test/metrics/histogram_tester.h" #include "base/test/mock_callback.h" @@ -19,6 +20,7 @@ #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkColor.h" #include "ui/gfx/codec/png_codec.h" +#include "ui/gfx/favicon_size.h" #include "ui/gfx/image/image.h" namespace favicon { @@ -27,47 +29,33 @@ namespace { using testing::_; using testing::Return; -// TODO(victorvianna): Rename to kPageUrl and kIconUrl. -const char kDummyPageUrl[] = "https://www.example.com"; -const char kDummyIconUrl[] = "https://www.example.com/favicon16.png"; -const HistoryUiFaviconRequestOrigin kDummyOrigin = - HistoryUiFaviconRequestOrigin::kHistory; -const char kDummyOriginHistogramSuffix[] = ".HISTORY"; -base::CancelableTaskTracker::TaskId kDummyTaskId = 1; -const char kAvailabilityHistogramName[] = - "Sync.SyncedHistoryFaviconAvailability"; -const char kLatencyHistogramName[] = "Sync.SyncedHistoryFaviconLatency"; -const int kDefaultDesiredSizeInPixel = 16; -const SkColor kTestColor = SK_ColorRED; +const base::CancelableTaskTracker::TaskId kTaskId = 1; SkBitmap CreateTestSkBitmap(int desired_size_in_pixel) { SkBitmap bitmap; bitmap.allocN32Pixels(desired_size_in_pixel, desired_size_in_pixel); - bitmap.eraseColor(kTestColor); + bitmap.eraseColor(SK_ColorRED); return bitmap; } favicon_base::FaviconRawBitmapResult CreateTestBitmapResult( + const GURL& icon_url, int desired_size_in_pixel) { scoped_refptr<base::RefCountedBytes> data(new base::RefCountedBytes()); gfx::PNGCodec::EncodeBGRASkBitmap(CreateTestSkBitmap(desired_size_in_pixel), false, &data->data()); favicon_base::FaviconRawBitmapResult result; result.bitmap_data = data; - result.icon_url = GURL(kDummyIconUrl); + result.icon_url = icon_url; result.pixel_size = gfx::Size(desired_size_in_pixel, desired_size_in_pixel); return result; } -favicon_base::FaviconRawBitmapResult CreateTestBitmapResult() { - return CreateTestBitmapResult(kDefaultDesiredSizeInPixel); -} - -favicon_base::FaviconImageResult CreateTestImageResult() { +favicon_base::FaviconImageResult CreateTestImageResult(const GURL& icon_url) { favicon_base::FaviconImageResult result; - result.image = gfx::Image::CreateFrom1xBitmap( - CreateTestSkBitmap(kDefaultDesiredSizeInPixel)); - result.icon_url = GURL(kDummyIconUrl); + result.image = + gfx::Image::CreateFrom1xBitmap(CreateTestSkBitmap(gfx::kFaviconSize)); + result.icon_url = icon_url; return result; } @@ -90,34 +78,38 @@ class MockFaviconServiceWithFake : public MockFaviconService { favicon_base::FaviconRawBitmapCallback callback, auto) { std::move(callback).Run(favicon_base::FaviconRawBitmapResult()); - return kDummyTaskId; + return kTaskId; }); ON_CALL(*this, GetFaviconImageForPageURL(_, _, _)) .WillByDefault( [](auto, favicon_base::FaviconImageCallback callback, auto) { std::move(callback).Run(favicon_base::FaviconImageResult()); - return kDummyTaskId; + return kTaskId; }); } ~MockFaviconServiceWithFake() override = default; - // Will make the object start responding with a valid favicon for |page_url|. - void StoreMockLocalFavicon(const GURL& page_url) { - ON_CALL(*this, GetRawFaviconForPageURL(GURL(page_url), _, - kDefaultDesiredSizeInPixel, _, _, _)) - .WillByDefault([](auto, auto, auto, auto, - favicon_base::FaviconRawBitmapCallback callback, - auto) { - std::move(callback).Run(CreateTestBitmapResult()); - return kDummyTaskId; - }); - ON_CALL(*this, GetFaviconImageForPageURL(GURL(page_url), _, _)) + // Simulates the service having an icon stored for |page_url|, the URL of the + // image being |icon_url|. The real FaviconService performs resizing if it + // can't find a stored icon matching the requested size, so the same is true + // here: any requested size will return a bitmap of that size. + void StoreMockLocalFavicon(const GURL& page_url, const GURL& icon_url) { + ON_CALL(*this, GetRawFaviconForPageURL(page_url, _, _, _, _, _)) .WillByDefault( - [](auto, favicon_base::FaviconImageCallback callback, auto) { - std::move(callback).Run(CreateTestImageResult()); - return kDummyTaskId; + [icon_url](auto, auto, int desired_size_in_pixel, auto, + favicon_base::FaviconRawBitmapCallback callback, auto) { + std::move(callback).Run( + CreateTestBitmapResult(icon_url, desired_size_in_pixel)); + return kTaskId; }); + ON_CALL(*this, GetFaviconImageForPageURL(page_url, _, _)) + .WillByDefault([icon_url](auto, + favicon_base::FaviconImageCallback callback, + auto) { + std::move(callback).Run(CreateTestImageResult(icon_url)); + return kTaskId; + }); } private: @@ -129,7 +121,7 @@ class MockLargeIconServiceWithFake : public LargeIconService { explicit MockLargeIconServiceWithFake( MockFaviconServiceWithFake* mock_favicon_service_with_fake) : mock_favicon_service_with_fake_(mock_favicon_service_with_fake) { - // Fake won't respond with any icons at first. + // Fake won't respond with any icons at first (HTTP error 404). ON_CALL(*this, GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache( _, _, _, _, _)) @@ -144,13 +136,31 @@ class MockLargeIconServiceWithFake : public LargeIconService { ~MockLargeIconServiceWithFake() override = default; + // Simulates the Google Server having an icon stored for |page_url|, of + // associated |icon_url|. Requests will cause the icon to be stored in + // |mock_favicon_service_with_fake_|. + void StoreMockGoogleServerFavicon(const GURL& page_url, + const GURL& icon_url) { + ON_CALL(*this, + GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache( + _, _, _, _, _)) + .WillByDefault( + [=](auto, auto, auto, auto, + favicon_base::GoogleFaviconServerCallback server_callback) { + mock_favicon_service_with_fake_->StoreMockLocalFavicon(page_url, + icon_url); + std::move(server_callback) + .Run(favicon_base::GoogleFaviconServerRequestStatus::SUCCESS); + }); + } + + // LargeIconService overrides. MOCK_METHOD5(GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache, void(const GURL& page_url, bool may_page_url_be_private, bool should_trim_page_url_path, const net::NetworkTrafficAnnotationTag& traffic_annotation, favicon_base::GoogleFaviconServerCallback callback)); - MOCK_METHOD5(GetLargeIconRawBitmapOrFallbackStyleForPageUrl, base::CancelableTaskTracker::TaskId( const GURL& page_url, @@ -158,7 +168,6 @@ class MockLargeIconServiceWithFake : public LargeIconService { int desired_size_in_pixel, favicon_base::LargeIconCallback callback, base::CancelableTaskTracker* tracker)); - MOCK_METHOD5(GetLargeIconImageOrFallbackStyleForPageUrl, base::CancelableTaskTracker::TaskId( const GURL& page_url, @@ -166,7 +175,6 @@ class MockLargeIconServiceWithFake : public LargeIconService { int desired_size_in_pixel, favicon_base::LargeIconImageCallback callback, base::CancelableTaskTracker* tracker)); - MOCK_METHOD5(GetLargeIconRawBitmapOrFallbackStyleForIconUrl, base::CancelableTaskTracker::TaskId( const GURL& icon_url, @@ -174,31 +182,14 @@ class MockLargeIconServiceWithFake : public LargeIconService { int desired_size_in_pixel, favicon_base::LargeIconCallback callback, base::CancelableTaskTracker* tracker)); - MOCK_METHOD4(GetIconRawBitmapOrFallbackStyleForPageUrl, base::CancelableTaskTracker::TaskId( const GURL& page_url, int desired_size_in_pixel, favicon_base::LargeIconCallback callback, base::CancelableTaskTracker* tracker)); - MOCK_METHOD1(TouchIconFromGoogleServer, void(const GURL& icon_url)); - // Will make the object respond by storing a valid local favicon for - // |page_url| in |mock_favicon_server_with_fake_|. - void StoreMockGoogleServerFavicon(const GURL& page_url) { - ON_CALL(*this, - GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache( - _, _, _, _, _)) - .WillByDefault( - [=](auto, auto, auto, auto, - favicon_base::GoogleFaviconServerCallback server_callback) { - mock_favicon_service_with_fake_->StoreMockLocalFavicon(page_url); - std::move(server_callback) - .Run(favicon_base::GoogleFaviconServerRequestStatus::SUCCESS); - }); - } - private: MockFaviconServiceWithFake* const mock_favicon_service_with_fake_; @@ -225,118 +216,122 @@ class HistoryUiFaviconRequestHandlerImplTest : public ::testing::Test { base::HistogramTester histogram_tester_; HistoryUiFaviconRequestHandlerImpl history_ui_favicon_request_handler_; - private: + // Convenience constants used in the tests. + const GURL kPageUrl = GURL("https://www.example.com"); + const GURL kIconUrl = GURL("https://www.example.com/favicon16.png"); + const HistoryUiFaviconRequestOrigin kOrigin = + HistoryUiFaviconRequestOrigin::kHistory; + const std::string kOriginHistogramSuffix = ".HISTORY"; + const std::string kAvailabilityHistogramName = + "Sync.SyncedHistoryFaviconAvailability"; + const std::string kLatencyHistogramName = "Sync.SyncedHistoryFaviconLatency"; + DISALLOW_COPY_AND_ASSIGN(HistoryUiFaviconRequestHandlerImplTest); }; TEST_F(HistoryUiFaviconRequestHandlerImplTest, ShouldGetEmptyBitmap) { EXPECT_CALL(mock_favicon_service_, - GetRawFaviconForPageURL(GURL(kDummyPageUrl), _, - kDefaultDesiredSizeInPixel, _, _, _)); + GetRawFaviconForPageURL(kPageUrl, _, + /*desired_size_in_pixel=*/16, _, _, _)); favicon_base::FaviconRawBitmapResult result; history_ui_favicon_request_handler_.GetRawFaviconForPageURL( - GURL(kDummyPageUrl), kDefaultDesiredSizeInPixel, - base::BindOnce(&StoreBitmap, &result), kDummyOrigin); + kPageUrl, /*desired_size_in_pixel=*/16, + base::BindOnce(&StoreBitmap, &result), kOrigin); EXPECT_FALSE(result.is_valid()); histogram_tester_.ExpectUniqueSample( - std::string(kAvailabilityHistogramName) + kDummyOriginHistogramSuffix, + kAvailabilityHistogramName + kOriginHistogramSuffix, FaviconAvailability::kNotAvailable, 1); histogram_tester_.ExpectTotalCount( - std::string(kLatencyHistogramName) + kDummyOriginHistogramSuffix, 1); + kLatencyHistogramName + kOriginHistogramSuffix, 1); } TEST_F(HistoryUiFaviconRequestHandlerImplTest, ShouldGetLocalBitmap) { - mock_favicon_service_.StoreMockLocalFavicon(GURL(kDummyPageUrl)); + mock_favicon_service_.StoreMockLocalFavicon(kPageUrl, kIconUrl); EXPECT_CALL(mock_favicon_service_, - GetRawFaviconForPageURL(GURL(kDummyPageUrl), _, - kDefaultDesiredSizeInPixel, _, _, _)); - EXPECT_CALL(mock_large_icon_service_, - TouchIconFromGoogleServer(GURL(kDummyIconUrl))); + GetRawFaviconForPageURL(kPageUrl, _, + /*desired_size_in_pixel=*/16, _, _, _)); + EXPECT_CALL(mock_large_icon_service_, TouchIconFromGoogleServer(kIconUrl)); favicon_base::FaviconRawBitmapResult result; history_ui_favicon_request_handler_.GetRawFaviconForPageURL( - GURL(kDummyPageUrl), kDefaultDesiredSizeInPixel, - base::BindOnce(&StoreBitmap, &result), kDummyOrigin); + kPageUrl, /*desired_size_in_pixel=*/16, + base::BindOnce(&StoreBitmap, &result), kOrigin); EXPECT_TRUE(result.is_valid()); histogram_tester_.ExpectUniqueSample( - std::string(kAvailabilityHistogramName) + kDummyOriginHistogramSuffix, + kAvailabilityHistogramName + kOriginHistogramSuffix, FaviconAvailability::kLocal, 1); histogram_tester_.ExpectTotalCount( - std::string(kLatencyHistogramName) + kDummyOriginHistogramSuffix, 1); + kLatencyHistogramName + kOriginHistogramSuffix, 1); } TEST_F(HistoryUiFaviconRequestHandlerImplTest, ShouldGetGoogleServerBitmap) { - mock_large_icon_service_.StoreMockGoogleServerFavicon(GURL(kDummyPageUrl)); + mock_large_icon_service_.StoreMockGoogleServerFavicon(kPageUrl, kIconUrl); EXPECT_CALL(can_send_history_data_getter_, Run()); EXPECT_CALL(mock_favicon_service_, - GetRawFaviconForPageURL(GURL(kDummyPageUrl), _, - kDefaultDesiredSizeInPixel, _, _, _)) + GetRawFaviconForPageURL(kPageUrl, _, + /*desired_size_in_pixel=*/16, _, _, _)) .Times(2); EXPECT_CALL(mock_large_icon_service_, GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache( - GURL(kDummyPageUrl), _, + kPageUrl, _, /*should_trim_url_path=*/false, _, _)); favicon_base::FaviconRawBitmapResult result; history_ui_favicon_request_handler_.GetRawFaviconForPageURL( - GURL(kDummyPageUrl), kDefaultDesiredSizeInPixel, - base::BindOnce(&StoreBitmap, &result), kDummyOrigin); + kPageUrl, /*desired_size_in_pixel=*/16, + base::BindOnce(&StoreBitmap, &result), kOrigin); EXPECT_TRUE(result.is_valid()); histogram_tester_.ExpectUniqueSample( - std::string(kAvailabilityHistogramName) + kDummyOriginHistogramSuffix, + kAvailabilityHistogramName + kOriginHistogramSuffix, FaviconAvailability::kLocal, 1); histogram_tester_.ExpectTotalCount( - std::string(kLatencyHistogramName) + kDummyOriginHistogramSuffix, 1); + kLatencyHistogramName + kOriginHistogramSuffix, 1); } TEST_F(HistoryUiFaviconRequestHandlerImplTest, ShouldGetEmptyImage) { - EXPECT_CALL(mock_favicon_service_, - GetFaviconImageForPageURL(GURL(kDummyPageUrl), _, _)); + EXPECT_CALL(mock_favicon_service_, GetFaviconImageForPageURL(kPageUrl, _, _)); favicon_base::FaviconImageResult result; history_ui_favicon_request_handler_.GetFaviconImageForPageURL( - GURL(kDummyPageUrl), base::BindOnce(&StoreImage, &result), kDummyOrigin); + kPageUrl, base::BindOnce(&StoreImage, &result), kOrigin); EXPECT_TRUE(result.image.IsEmpty()); histogram_tester_.ExpectUniqueSample( - std::string(kAvailabilityHistogramName) + kDummyOriginHistogramSuffix, + kAvailabilityHistogramName + kOriginHistogramSuffix, FaviconAvailability::kNotAvailable, 1); histogram_tester_.ExpectTotalCount( - std::string(kLatencyHistogramName) + kDummyOriginHistogramSuffix, 1); + kLatencyHistogramName + kOriginHistogramSuffix, 1); } TEST_F(HistoryUiFaviconRequestHandlerImplTest, ShouldGetLocalImage) { - mock_favicon_service_.StoreMockLocalFavicon(GURL(kDummyPageUrl)); - EXPECT_CALL(mock_favicon_service_, - GetFaviconImageForPageURL(GURL(kDummyPageUrl), _, _)); - EXPECT_CALL(mock_large_icon_service_, - TouchIconFromGoogleServer(GURL(kDummyIconUrl))); + mock_favicon_service_.StoreMockLocalFavicon(kPageUrl, kIconUrl); + EXPECT_CALL(mock_favicon_service_, GetFaviconImageForPageURL(kPageUrl, _, _)); + EXPECT_CALL(mock_large_icon_service_, TouchIconFromGoogleServer(kIconUrl)); favicon_base::FaviconImageResult result; history_ui_favicon_request_handler_.GetFaviconImageForPageURL( - GURL(kDummyPageUrl), base::BindOnce(&StoreImage, &result), kDummyOrigin); + kPageUrl, base::BindOnce(&StoreImage, &result), kOrigin); EXPECT_FALSE(result.image.IsEmpty()); histogram_tester_.ExpectUniqueSample( - std::string(kAvailabilityHistogramName) + kDummyOriginHistogramSuffix, + kAvailabilityHistogramName + kOriginHistogramSuffix, FaviconAvailability::kLocal, 1); histogram_tester_.ExpectTotalCount( - std::string(kLatencyHistogramName) + kDummyOriginHistogramSuffix, 1); + kLatencyHistogramName + kOriginHistogramSuffix, 1); } TEST_F(HistoryUiFaviconRequestHandlerImplTest, ShouldGetGoogleServerImage) { - mock_large_icon_service_.StoreMockGoogleServerFavicon(GURL(kDummyPageUrl)); + mock_large_icon_service_.StoreMockGoogleServerFavicon(kPageUrl, kIconUrl); EXPECT_CALL(can_send_history_data_getter_, Run()); - EXPECT_CALL(mock_favicon_service_, - GetFaviconImageForPageURL(GURL(kDummyPageUrl), _, _)) + EXPECT_CALL(mock_favicon_service_, GetFaviconImageForPageURL(kPageUrl, _, _)) .Times(2); EXPECT_CALL(mock_large_icon_service_, GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache( - GURL(kDummyPageUrl), _, + kPageUrl, _, /*should_trim_url_path=*/false, _, _)); favicon_base::FaviconImageResult result; history_ui_favicon_request_handler_.GetFaviconImageForPageURL( - GURL(kDummyPageUrl), base::BindOnce(&StoreImage, &result), kDummyOrigin); + kPageUrl, base::BindOnce(&StoreImage, &result), kOrigin); EXPECT_FALSE(result.image.IsEmpty()); histogram_tester_.ExpectUniqueSample( - std::string(kAvailabilityHistogramName) + kDummyOriginHistogramSuffix, + kAvailabilityHistogramName + kOriginHistogramSuffix, FaviconAvailability::kLocal, 1); histogram_tester_.ExpectTotalCount( - std::string(kLatencyHistogramName) + kDummyOriginHistogramSuffix, 1); + kLatencyHistogramName + kOriginHistogramSuffix, 1); } TEST_F(HistoryUiFaviconRequestHandlerImplTest, @@ -345,21 +340,19 @@ TEST_F(HistoryUiFaviconRequestHandlerImplTest, return false; }); EXPECT_CALL(mock_favicon_service_, - GetRawFaviconForPageURL(GURL(kDummyPageUrl), _, - kDefaultDesiredSizeInPixel, _, _, _)) + GetRawFaviconForPageURL(kPageUrl, _, + /*desired_size_in_pixel=*/16, _, _, _)) .WillOnce([](auto, auto, auto, auto, favicon_base::FaviconRawBitmapCallback callback, auto) { std::move(callback).Run(favicon_base::FaviconRawBitmapResult()); - return kDummyTaskId; + return kTaskId; }); EXPECT_CALL(mock_large_icon_service_, GetLargeIconOrFallbackStyleFromGoogleServerSkippingLocalCache( _, _, _, _, _)) .Times(0); - favicon_base::FaviconRawBitmapResult result; history_ui_favicon_request_handler_.GetRawFaviconForPageURL( - GURL(kDummyPageUrl), kDefaultDesiredSizeInPixel, - base::BindOnce(&StoreBitmap, &result), kDummyOrigin); + kPageUrl, /*desired_size_in_pixel=*/16, base::DoNothing(), kOrigin); } } // namespace diff --git a/chromium/components/favicon/core/test/BUILD.gn b/chromium/components/favicon/core/test/BUILD.gn index d87927d2d85..f3013728a91 100644 --- a/chromium/components/favicon/core/test/BUILD.gn +++ b/chromium/components/favicon/core/test/BUILD.gn @@ -6,6 +6,8 @@ static_library("test_support") { testonly = true sources = [ + "favicon_driver_impl_test_helper.cc", + "favicon_driver_impl_test_helper.h", "mock_favicon_service.cc", "mock_favicon_service.h", ] diff --git a/chromium/components/favicon/ios/favicon_url_util.cc b/chromium/components/favicon/ios/favicon_url_util.cc index a0eb5e1dca0..35c429efab1 100644 --- a/chromium/components/favicon/ios/favicon_url_util.cc +++ b/chromium/components/favicon/ios/favicon_url_util.cc @@ -7,6 +7,7 @@ #include <algorithm> #include <iterator> +#include "base/notreached.h" #include "components/favicon/core/favicon_url.h" #include "components/favicon_base/favicon_types.h" #include "ios/web/public/favicon/favicon_url.h" |