Commit 2589a7a5 authored by Tommy C. Li's avatar Tommy C. Li Committed by Commit Bot

Search Engines: Update the search provider favicons at runtime

Currently, the search providers all have static prepopulated favicon
URLs. These can fall out of date.

This CL updates the search providers' favicons as the user
browses. Specifically, when the user actually lands on a URL matching
the a search provider URL template, and we get a favicon that does not
match the stored favicon URL, we update the stored favicon URL.

Bug: 88243, 823535
Change-Id: I43f72ad449515d5a19e5ec4b32ca996b9d32eb11
Reviewed-on: https://chromium-review.googlesource.com/1048174
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557742}
parent a4d648a1
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <memory> #include <memory>
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "chrome/browser/favicon/favicon_utils.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search_engines/template_url_fetcher_factory.h" #include "chrome/browser/search_engines/template_url_fetcher_factory.h"
#include "chrome/browser/search_engines/template_url_service_factory.h" #include "chrome/browser/search_engines/template_url_service_factory.h"
...@@ -88,10 +89,18 @@ void SearchEngineTabHelper::DidFinishNavigation( ...@@ -88,10 +89,18 @@ void SearchEngineTabHelper::DidFinishNavigation(
GenerateKeywordIfNecessary(handle); GenerateKeywordIfNecessary(handle);
} }
void SearchEngineTabHelper::WebContentsDestroyed() {
favicon_driver_observer_.RemoveAll();
}
SearchEngineTabHelper::SearchEngineTabHelper(WebContents* web_contents) SearchEngineTabHelper::SearchEngineTabHelper(WebContents* web_contents)
: content::WebContentsObserver(web_contents), : content::WebContentsObserver(web_contents),
osdd_handler_bindings_(web_contents, this) { osdd_handler_bindings_(web_contents, this) {
DCHECK(web_contents); DCHECK(web_contents);
favicon::CreateContentFaviconDriverForWebContents(web_contents);
favicon_driver_observer_.Add(
favicon::ContentFaviconDriver::FromWebContents(web_contents));
} }
void SearchEngineTabHelper::PageHasOpenSearchDescriptionDocument( void SearchEngineTabHelper::PageHasOpenSearchDescriptionDocument(
...@@ -144,6 +153,20 @@ void SearchEngineTabHelper::PageHasOpenSearchDescriptionDocument( ...@@ -144,6 +153,20 @@ void SearchEngineTabHelper::PageHasOpenSearchDescriptionDocument(
base::Bind(&AssociateURLFetcherWithWebContents, web_contents())); base::Bind(&AssociateURLFetcherWithWebContents, web_contents()));
} }
void SearchEngineTabHelper::OnFaviconUpdated(
favicon::FaviconDriver* driver,
NotificationIconType notification_icon_type,
const GURL& icon_url,
bool icon_url_changed,
const gfx::Image& image) {
Profile* profile =
Profile::FromBrowserContext(web_contents()->GetBrowserContext());
TemplateURLService* url_service =
TemplateURLServiceFactory::GetForProfile(profile);
if (url_service && url_service->loaded())
url_service->UpdateProviderFavicons(driver->GetActiveURL(), icon_url);
}
void SearchEngineTabHelper::GenerateKeywordIfNecessary( void SearchEngineTabHelper::GenerateKeywordIfNecessary(
content::NavigationHandle* handle) { content::NavigationHandle* handle) {
if (!handle->IsInMainFrame() || !handle->GetSearchableFormURL().is_valid()) if (!handle->IsInMainFrame() || !handle->GetSearchableFormURL().is_valid())
......
...@@ -6,9 +6,11 @@ ...@@ -6,9 +6,11 @@
#define CHROME_BROWSER_UI_SEARCH_ENGINES_SEARCH_ENGINE_TAB_HELPER_H_ #define CHROME_BROWSER_UI_SEARCH_ENGINES_SEARCH_ENGINE_TAB_HELPER_H_
#include "base/macros.h" #include "base/macros.h"
#include "base/scoped_observer.h"
#include "chrome/browser/ui/find_bar/find_bar_controller.h" #include "chrome/browser/ui/find_bar/find_bar_controller.h"
#include "chrome/browser/ui/find_bar/find_notification_details.h" #include "chrome/browser/ui/find_bar/find_notification_details.h"
#include "chrome/common/open_search_description_document_handler.mojom.h" #include "chrome/common/open_search_description_document_handler.mojom.h"
#include "components/favicon/core/favicon_driver_observer.h"
#include "content/public/browser/web_contents_binding_set.h" #include "content/public/browser/web_contents_binding_set.h"
#include "content/public/browser/web_contents_observer.h" #include "content/public/browser/web_contents_observer.h"
#include "content/public/browser/web_contents_user_data.h" #include "content/public/browser/web_contents_user_data.h"
...@@ -18,12 +20,14 @@ ...@@ -18,12 +20,14 @@
class SearchEngineTabHelper class SearchEngineTabHelper
: public content::WebContentsObserver, : public content::WebContentsObserver,
public content::WebContentsUserData<SearchEngineTabHelper>, public content::WebContentsUserData<SearchEngineTabHelper>,
public chrome::mojom::OpenSearchDescriptionDocumentHandler { public chrome::mojom::OpenSearchDescriptionDocumentHandler,
public favicon::FaviconDriverObserver {
public: public:
~SearchEngineTabHelper() override; ~SearchEngineTabHelper() override;
// content::WebContentsObserver overrides. // content::WebContentsObserver overrides.
void DidFinishNavigation(content::NavigationHandle* handle) override; void DidFinishNavigation(content::NavigationHandle* handle) override;
void WebContentsDestroyed() override;
private: private:
explicit SearchEngineTabHelper(content::WebContents* web_contents); explicit SearchEngineTabHelper(content::WebContents* web_contents);
...@@ -33,6 +37,13 @@ class SearchEngineTabHelper ...@@ -33,6 +37,13 @@ class SearchEngineTabHelper
void PageHasOpenSearchDescriptionDocument(const GURL& page_url, void PageHasOpenSearchDescriptionDocument(const GURL& page_url,
const GURL& osdd_url) override; const GURL& osdd_url) override;
// favicon::FaviconDriverObserver:
void OnFaviconUpdated(favicon::FaviconDriver* driver,
NotificationIconType notification_icon_type,
const GURL& icon_url,
bool icon_url_changed,
const gfx::Image& image) override;
// If params has a searchable form, this tries to create a new keyword. // If params has a searchable form, this tries to create a new keyword.
void GenerateKeywordIfNecessary(content::NavigationHandle* handle); void GenerateKeywordIfNecessary(content::NavigationHandle* handle);
...@@ -40,6 +51,9 @@ class SearchEngineTabHelper ...@@ -40,6 +51,9 @@ class SearchEngineTabHelper
chrome::mojom::OpenSearchDescriptionDocumentHandler> chrome::mojom::OpenSearchDescriptionDocumentHandler>
osdd_handler_bindings_; osdd_handler_bindings_;
ScopedObserver<favicon::FaviconDriver, favicon::FaviconDriverObserver>
favicon_driver_observer_{this};
DISALLOW_COPY_AND_ASSIGN(SearchEngineTabHelper); DISALLOW_COPY_AND_ASSIGN(SearchEngineTabHelper);
}; };
......
...@@ -227,6 +227,7 @@ void DefaultSearchManager::MergePrefsDataWithPrepopulated() { ...@@ -227,6 +227,7 @@ void DefaultSearchManager::MergePrefsDataWithPrepopulated() {
engine->date_created = prefs_default_search_->date_created; engine->date_created = prefs_default_search_->date_created;
engine->last_modified = prefs_default_search_->last_modified; engine->last_modified = prefs_default_search_->last_modified;
engine->last_visited = prefs_default_search_->last_visited; engine->last_visited = prefs_default_search_->last_visited;
engine->favicon_url = prefs_default_search_->favicon_url;
prefs_default_search_ = std::move(engine); prefs_default_search_ = std::move(engine);
} }
......
...@@ -1304,7 +1304,6 @@ bool TemplateURL::MatchesData(const TemplateURL* t_url, ...@@ -1304,7 +1304,6 @@ bool TemplateURL::MatchesData(const TemplateURL* t_url,
(t_url->suggestions_url_post_params() == (t_url->suggestions_url_post_params() ==
data->suggestions_url_post_params) && data->suggestions_url_post_params) &&
(t_url->image_url_post_params() == data->image_url_post_params) && (t_url->image_url_post_params() == data->image_url_post_params) &&
(t_url->favicon_url() == data->favicon_url) &&
(t_url->safe_for_autoreplace() == data->safe_for_autoreplace) && (t_url->safe_for_autoreplace() == data->safe_for_autoreplace) &&
(t_url->input_encodings() == data->input_encodings) && (t_url->input_encodings() == data->input_encodings) &&
(t_url->alternate_urls() == data->alternate_urls); (t_url->alternate_urls() == data->alternate_urls);
......
...@@ -583,6 +583,29 @@ void TemplateURLService::ResetTemplateURL(TemplateURL* url, ...@@ -583,6 +583,29 @@ void TemplateURLService::ResetTemplateURL(TemplateURL* url,
Update(url, TemplateURL(data)); Update(url, TemplateURL(data));
} }
void TemplateURLService::UpdateProviderFavicons(
const GURL& potential_search_url,
const GURL& favicon_url) {
DCHECK(loaded_);
DCHECK(potential_search_url.is_valid());
const TemplateURLSet* urls_for_host =
provider_map_->GetURLsForHost(potential_search_url.host());
if (!urls_for_host)
return;
Scoper scoper(this);
for (TemplateURL* turl : *urls_for_host) {
if (!IsCreatedByExtension(turl) &&
turl->IsSearchURL(potential_search_url, search_terms_data()) &&
turl->favicon_url() != favicon_url) {
TemplateURLData data(turl->data());
data.favicon_url = favicon_url;
Update(turl, TemplateURL(data));
}
}
}
bool TemplateURLService::CanMakeDefault(const TemplateURL* url) const { bool TemplateURLService::CanMakeDefault(const TemplateURL* url) const {
return return
((default_search_provider_source_ == DefaultSearchManager::FROM_USER) || ((default_search_provider_source_ == DefaultSearchManager::FROM_USER) ||
......
...@@ -238,6 +238,11 @@ class TemplateURLService : public WebDataServiceConsumer, ...@@ -238,6 +238,11 @@ class TemplateURLService : public WebDataServiceConsumer,
const base::string16& keyword, const base::string16& keyword,
const std::string& search_url); const std::string& search_url);
// Updates any search providers matching |potential_search_url| with the new
// favicon location |favicon_url|.
void UpdateProviderFavicons(const GURL& potential_search_url,
const GURL& favicon_url);
// Return true if the given |url| can be made the default. This returns false // Return true if the given |url| can be made the default. This returns false
// regardless of |url| if the default search provider is managed by policy or // regardless of |url| if the default search provider is managed by policy or
// controlled by an extension. // controlled by an extension.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment