Add favicon url mapping interface
We introduce an interface for mapping a favicon's icon url to page url.
We implement that interface in FaviconCache using information obtained
from sessions sync.
Bug: 955475
Change-Id: Idc3f31c686902cd673af7e84e36509e5dbe1f4d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1578567
Auto-Submit: Victor Vianna <victorvianna@google.com>
Reviewed-by: Sergio Collazos <sczs@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/master@{#662104}
diff --git a/components/favicon/core/BUILD.gn b/components/favicon/core/BUILD.gn
index bb3a731b0..b9e3a01 100644
--- a/components/favicon/core/BUILD.gn
+++ b/components/favicon/core/BUILD.gn
@@ -23,6 +23,7 @@
"favicon_service_impl.h",
"favicon_url.cc",
"favicon_url.h",
+ "favicon_url_mapper.h",
"favicon_util.cc",
"favicon_util.h",
"features.cc",
diff --git a/components/favicon/core/favicon_url_mapper.h b/components/favicon/core/favicon_url_mapper.h
new file mode 100644
index 0000000..4b49a2b
--- /dev/null
+++ b/components/favicon/core/favicon_url_mapper.h
@@ -0,0 +1,24 @@
+// Copyright 2019 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.
+
+#ifndef COMPONENTS_FAVICON_CORE_FAVICON_URL_MAPPER_H_
+#define COMPONENTS_FAVICON_CORE_FAVICON_URL_MAPPER_H_
+
+#include "url/gurl.h"
+
+namespace favicon {
+
+// Interface for the translation of a page URL to the URL of the favicon the
+// browser associated to this page.
+class FaviconUrlMapper {
+ public:
+ virtual ~FaviconUrlMapper() = default;
+ // Returns the corresponding icon url if such exists, otherwise returns the
+ // empty GURL.
+ virtual GURL GetIconUrlForPageUrl(const GURL& page_url) = 0;
+};
+
+} // namespace favicon
+
+#endif // COMPONENTS_FAVICON_CORE_FAVICON_URL_MAPPER_H_
diff --git a/components/sync_sessions/favicon_cache.cc b/components/sync_sessions/favicon_cache.cc
index ce4170b2d..6aa679cf 100644
--- a/components/sync_sessions/favicon_cache.cc
+++ b/components/sync_sessions/favicon_cache.cc
@@ -504,12 +504,18 @@
scoped_refptr<base::RefCountedMemory>* favicon_png) const {
if (!page_url.is_valid())
return false;
- auto iter = page_favicon_map_.find(page_url);
-
- if (iter == page_favicon_map_.end())
+ GURL icon_url = GetIconUrlForPageUrl(page_url);
+ if (icon_url.is_empty())
return false;
- return GetSyncedFaviconForFaviconURL(iter->second, favicon_png);
+ return GetSyncedFaviconForFaviconURL(icon_url, favicon_png);
+}
+
+GURL FaviconCache::GetIconUrlForPageUrl(const GURL& page_url) const {
+ auto iter = page_favicon_map_.find(page_url);
+ if (iter == page_favicon_map_.end())
+ return GURL();
+ return iter->second;
}
void FaviconCache::UpdateMappingsFromForeignTab(const sync_pb::SessionTab& tab,
diff --git a/components/sync_sessions/favicon_cache.h b/components/sync_sessions/favicon_cache.h
index 4d2c456..da20c16 100644
--- a/components/sync_sessions/favicon_cache.h
+++ b/components/sync_sessions/favicon_cache.h
@@ -85,6 +85,10 @@
const GURL& favicon_url,
scoped_refptr<base::RefCountedMemory>* favicon_png) const;
+ // Returns the value associated with |page_url| in |page_favicon_map_| if one
+ // exists, otherwise returns an empty URL.
+ GURL GetIconUrlForPageUrl(const GURL& page_url) const;
+
// If a valid favicon for the icon associated with |page_url| is found, fills
// |favicon_png| with the png-encoded image and returns true. Else, returns
// false.
diff --git a/components/sync_sessions/open_tabs_ui_delegate.h b/components/sync_sessions/open_tabs_ui_delegate.h
index de915e1..47995ae1 100644
--- a/components/sync_sessions/open_tabs_ui_delegate.h
+++ b/components/sync_sessions/open_tabs_ui_delegate.h
@@ -10,13 +10,14 @@
#include "base/memory/ref_counted.h"
#include "base/memory/ref_counted_memory.h"
+#include "components/favicon/core/favicon_url_mapper.h"
#include "components/sessions/core/session_id.h"
#include "components/sessions/core/session_types.h"
#include "components/sync_sessions/synced_session.h"
namespace sync_sessions {
-class OpenTabsUIDelegate {
+class OpenTabsUIDelegate : public favicon::FaviconUrlMapper {
public:
// If a valid favicon for the page at |url| is found, fills |favicon_png| with
// the png-encoded image and returns true. Else, returns false.
@@ -61,7 +62,7 @@
virtual bool GetLocalSession(const SyncedSession** local) = 0;
protected:
- virtual ~OpenTabsUIDelegate();
+ ~OpenTabsUIDelegate() override;
};
} // namespace sync_sessions
diff --git a/components/sync_sessions/open_tabs_ui_delegate_impl.cc b/components/sync_sessions/open_tabs_ui_delegate_impl.cc
index 4d7e14a..16f833ab 100644
--- a/components/sync_sessions/open_tabs_ui_delegate_impl.cc
+++ b/components/sync_sessions/open_tabs_ui_delegate_impl.cc
@@ -106,4 +106,8 @@
return *local_session != nullptr;
}
+GURL OpenTabsUIDelegateImpl::GetIconUrlForPageUrl(const GURL& page_url) {
+ return favicon_cache_->GetIconUrlForPageUrl(page_url);
+}
+
} // namespace sync_sessions
diff --git a/components/sync_sessions/open_tabs_ui_delegate_impl.h b/components/sync_sessions/open_tabs_ui_delegate_impl.h
index e8b1d9d..df043a4b 100644
--- a/components/sync_sessions/open_tabs_ui_delegate_impl.h
+++ b/components/sync_sessions/open_tabs_ui_delegate_impl.h
@@ -48,6 +48,7 @@
std::vector<const sessions::SessionTab*>* tabs) override;
void DeleteForeignSession(const std::string& tag) override;
bool GetLocalSession(const SyncedSession** local_session) override;
+ GURL GetIconUrlForPageUrl(const GURL& page_url) override;
private:
const SyncSessionsClient* const sessions_client_;
diff --git a/ios/chrome/browser/ui/recent_tabs/recent_tabs_coordinator_unittest.mm b/ios/chrome/browser/ui/recent_tabs/recent_tabs_coordinator_unittest.mm
index 8f20e0c..55da528 100644
--- a/ios/chrome/browser/ui/recent_tabs/recent_tabs_coordinator_unittest.mm
+++ b/ios/chrome/browser/ui/recent_tabs/recent_tabs_coordinator_unittest.mm
@@ -30,6 +30,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#import "third_party/ocmock/OCMock/OCMock.h"
#import "third_party/ocmock/gtest_support.h"
+#include "url/gurl.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
@@ -82,6 +83,8 @@
OpenTabsUIDelegateMock() {}
~OpenTabsUIDelegateMock() override {}
+ MOCK_METHOD1(GetIconUrlForPageUrl, GURL(const GURL& page_url));
+
MOCK_CONST_METHOD2(GetSyncedFaviconForPageURL,
bool(const std::string& pageurl,
scoped_refptr<base::RefCountedMemory>* favicon_png));