diff options
-rw-r--r-- | chrome/browser/android/tab_android.cc | 4 | ||||
-rw-r--r-- | chrome/browser/android/tab_android.h | 2 | ||||
-rw-r--r-- | chrome/browser/chrome_notification_types.h | 10 | ||||
-rw-r--r-- | chrome/browser/extensions/api/tabs/tabs_event_router.cc | 30 | ||||
-rw-r--r-- | chrome/browser/extensions/api/tabs/tabs_event_router.h | 12 | ||||
-rw-r--r-- | chrome/browser/favicon/favicon_handler_unittest.cc | 2 | ||||
-rw-r--r-- | chrome/browser/favicon/favicon_tab_helper.cc | 14 | ||||
-rw-r--r-- | chrome/browser/favicon/favicon_tab_helper.h | 1 | ||||
-rw-r--r-- | chrome/browser/favicon/favicon_tab_helper_browsertest.cc | 29 | ||||
-rw-r--r-- | chrome/browser/prerender/prerender_browsertest.cc | 43 | ||||
-rw-r--r-- | components/favicon/core/favicon_driver.h | 5 | ||||
-rw-r--r-- | components/favicon/core/favicon_driver_observer.h | 7 |
12 files changed, 123 insertions, 36 deletions
diff --git a/chrome/browser/android/tab_android.cc b/chrome/browser/android/tab_android.cc index 120ad48..2b7a974 100644 --- a/chrome/browser/android/tab_android.cc +++ b/chrome/browser/android/tab_android.cc @@ -395,6 +395,10 @@ void TabAndroid::OnFaviconAvailable(const gfx::Image& image) { gfx::ConvertToJavaBitmap(&favicon).obj()); } +void TabAndroid::OnFaviconUpdated(favicon::FaviconDriver* favicon_driver, + bool icon_url_changed) { +} + void TabAndroid::Destroy(JNIEnv* env, jobject obj) { delete this; } diff --git a/chrome/browser/android/tab_android.h b/chrome/browser/android/tab_android.h index af89b58..241505e 100644 --- a/chrome/browser/android/tab_android.h +++ b/chrome/browser/android/tab_android.h @@ -139,6 +139,8 @@ class TabAndroid : public CoreTabHelperDelegate, // favicon::FaviconDriverObserver ------------------------------------------- void OnFaviconAvailable(const gfx::Image& image) override; + void OnFaviconUpdated(favicon::FaviconDriver* favicon_driver, + bool icon_url_changed) override; // Methods called from Java via JNI ----------------------------------------- diff --git a/chrome/browser/chrome_notification_types.h b/chrome/browser/chrome_notification_types.h index e7938209..e40a7e0 100644 --- a/chrome/browser/chrome_notification_types.h +++ b/chrome/browser/chrome_notification_types.h @@ -228,16 +228,6 @@ enum NotificationType { // the LoginHandler that should be cancelled. NOTIFICATION_AUTH_CANCELLED, - // Favicon ------------------------------------------------------------------ - - // Sent by FaviconTabHelper when a tab's favicon has been successfully - // updated. The details are a bool indicating whether the - // NavigationEntry's favicon URL has changed since the previous - // NOTIFICATION_FAVICON_UPDATED notification. The details are true if - // there was no previous NOTIFICATION_FAVICON_UPDATED notification for the - // current NavigationEntry. - NOTIFICATION_FAVICON_UPDATED, - // Profiles ----------------------------------------------------------------- // Sent after a Profile has been created. This notification is sent both for diff --git a/chrome/browser/extensions/api/tabs/tabs_event_router.cc b/chrome/browser/extensions/api/tabs/tabs_event_router.cc index baab0e1..8cbd534 100644 --- a/chrome/browser/extensions/api/tabs/tabs_event_router.cc +++ b/chrome/browser/extensions/api/tabs/tabs_event_router.cc @@ -11,6 +11,7 @@ #include "chrome/browser/extensions/api/tabs/tabs_windows_api.h" #include "chrome/browser/extensions/api/tabs/windows_event_router.h" #include "chrome/browser/extensions/extension_tab_util.h" +#include "chrome/browser/favicon/favicon_tab_helper.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_iterator.h" @@ -94,7 +95,8 @@ base::DictionaryValue* TabsEventRouter::TabEntry::DidNavigate( return changed_properties; } -TabsEventRouter::TabsEventRouter(Profile* profile) : profile_(profile) { +TabsEventRouter::TabsEventRouter(Profile* profile) + : profile_(profile), favicon_scoped_observer_(this) { DCHECK(!profile->IsOffTheRecord()); BrowserList::AddObserver(this); @@ -148,8 +150,7 @@ void TabsEventRouter::RegisterForTabNotifications(WebContents* contents) { registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED, content::Source<WebContents>(contents)); - registrar_.Add(this, chrome::NOTIFICATION_FAVICON_UPDATED, - content::Source<WebContents>(contents)); + favicon_scoped_observer_.Add(FaviconTabHelper::FromWebContents(contents)); ZoomController::FromWebContents(contents)->AddObserver(this); } @@ -159,8 +160,7 @@ void TabsEventRouter::UnregisterForTabNotifications(WebContents* contents) { content::Source<NavigationController>(&contents->GetController())); registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED, content::Source<WebContents>(contents)); - registrar_.Remove(this, chrome::NOTIFICATION_FAVICON_UPDATED, - content::Source<WebContents>(contents)); + favicon_scoped_observer_.Remove(FaviconTabHelper::FromWebContents(contents)); ZoomController::FromWebContents(contents)->RemoveObserver(this); } @@ -501,12 +501,8 @@ void TabsEventRouter::Observe(int type, content::Source<NavigationController>(&contents->GetController())); registrar_.Remove(this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED, content::Source<WebContents>(contents)); - registrar_.Remove(this, chrome::NOTIFICATION_FAVICON_UPDATED, - content::Source<WebContents>(contents)); - } else if (type == chrome::NOTIFICATION_FAVICON_UPDATED) { - bool icon_url_changed = *content::Details<bool>(details).ptr(); - if (icon_url_changed) - FaviconUrlUpdated(content::Source<WebContents>(source).ptr()); + favicon_scoped_observer_.Remove( + FaviconTabHelper::FromWebContents(contents)); } else { NOTREACHED(); } @@ -585,4 +581,16 @@ void TabsEventRouter::OnZoomChanged( EventRouter::USER_GESTURE_UNKNOWN); } +void TabsEventRouter::OnFaviconAvailable(const gfx::Image& image) { +} + +void TabsEventRouter::OnFaviconUpdated(favicon::FaviconDriver* favicon_driver, + bool icon_url_changed) { + if (icon_url_changed) { + FaviconTabHelper* favicon_tab_helper = + static_cast<FaviconTabHelper*>(favicon_driver); + FaviconUrlUpdated(favicon_tab_helper->web_contents()); + } +} + } // namespace extensions diff --git a/chrome/browser/extensions/api/tabs/tabs_event_router.h b/chrome/browser/extensions/api/tabs/tabs_event_router.h index 73ee2c9..501c5fd 100644 --- a/chrome/browser/extensions/api/tabs/tabs_event_router.h +++ b/chrome/browser/extensions/api/tabs/tabs_event_router.h @@ -10,13 +10,17 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" +#include "base/scoped_observer.h" #include "chrome/browser/extensions/api/tabs/tabs_api.h" #include "chrome/browser/ui/browser_list_observer.h" #include "chrome/browser/ui/tabs/tab_strip_model_observer.h" +#include "components/favicon/core/favicon_driver_observer.h" #include "components/ui/zoom/zoom_observer.h" #include "content/public/browser/notification_registrar.h" #include "extensions/browser/event_router.h" +class FaviconTabHelper; + namespace content { class WebContents; } @@ -30,6 +34,7 @@ namespace extensions { class TabsEventRouter : public TabStripModelObserver, public chrome::BrowserListObserver, public content::NotificationObserver, + public favicon::FaviconDriverObserver, public ui_zoom::ZoomObserver { public: explicit TabsEventRouter(Profile* profile); @@ -76,6 +81,11 @@ class TabsEventRouter : public TabStripModelObserver, void OnZoomChanged( const ui_zoom::ZoomController::ZoomChangedEventData& data) override; + // favicon::FaviconDriverObserver. + void OnFaviconAvailable(const gfx::Image& image) override; + void OnFaviconUpdated(favicon::FaviconDriver* favicon_driver, + bool icon_url_changed) override; + private: // "Synthetic" event. Called from TabInsertedAt if new tab is detected. void TabCreatedAt(content::WebContents* contents, int index, bool active); @@ -169,6 +179,8 @@ class TabsEventRouter : public TabStripModelObserver, // The main profile that owns this event router. Profile* profile_; + ScopedObserver<FaviconTabHelper, TabsEventRouter> favicon_scoped_observer_; + DISALLOW_COPY_AND_ASSIGN(TabsEventRouter); }; diff --git a/chrome/browser/favicon/favicon_handler_unittest.cc b/chrome/browser/favicon/favicon_handler_unittest.cc index 2e858d5..5a70ff4 100644 --- a/chrome/browser/favicon/favicon_handler_unittest.cc +++ b/chrome/browser/favicon/favicon_handler_unittest.cc @@ -231,6 +231,8 @@ class TestFaviconDriver : public favicon::FaviconDriver { SetActiveFaviconImage(image); } + void NotifyFaviconUpdated(bool icon_url_changed) override {} + size_t num_active_favicon() const { return num_active_favicon_; } size_t num_favicon_available() const { return num_favicon_available_; } void ResetNumActiveFavicon() { num_active_favicon_ = 0; } diff --git a/chrome/browser/favicon/favicon_tab_helper.cc b/chrome/browser/favicon/favicon_tab_helper.cc index 8832076..e5e9334 100644 --- a/chrome/browser/favicon/favicon_tab_helper.cc +++ b/chrome/browser/favicon/favicon_tab_helper.cc @@ -268,20 +268,16 @@ void FaviconTabHelper::OnFaviconAvailable(const gfx::Image& image, const GURL& icon_url, bool is_active_favicon) { if (is_active_favicon) { + bool icon_url_changed = GetActiveFaviconURL() != icon_url; // No matter what happens, we need to mark the favicon as being set. SetActiveFaviconValidity(true); - bool icon_url_changed = GetActiveFaviconURL() != icon_url; SetActiveFaviconURL(icon_url); if (image.IsEmpty()) return; SetActiveFaviconImage(image); - content::NotificationService::current()->Notify( - chrome::NOTIFICATION_FAVICON_UPDATED, - content::Source<WebContents>(web_contents()), - content::Details<bool>(&icon_url_changed)); - web_contents()->NotifyNavigationStateChanged(content::INVALIDATE_TYPE_TAB); + NotifyFaviconUpdated(icon_url_changed); } if (!image.IsEmpty()) { FOR_EACH_OBSERVER(favicon::FaviconDriverObserver, observer_list_, @@ -289,6 +285,12 @@ void FaviconTabHelper::OnFaviconAvailable(const gfx::Image& image, } } +void FaviconTabHelper::NotifyFaviconUpdated(bool icon_url_changed) { + FOR_EACH_OBSERVER(favicon::FaviconDriverObserver, observer_list_, + OnFaviconUpdated(this, icon_url_changed)); + web_contents()->NotifyNavigationStateChanged(content::INVALIDATE_TYPE_TAB); +} + void FaviconTabHelper::DidDownloadFavicon( int id, int http_status_code, diff --git a/chrome/browser/favicon/favicon_tab_helper.h b/chrome/browser/favicon/favicon_tab_helper.h index 8c3e586..537ee40 100644 --- a/chrome/browser/favicon/favicon_tab_helper.h +++ b/chrome/browser/favicon/favicon_tab_helper.h @@ -102,6 +102,7 @@ class FaviconTabHelper : public content::WebContentsObserver, void OnFaviconAvailable(const gfx::Image& image, const GURL& url, bool is_active_favicon) override; + void NotifyFaviconUpdated(bool icon_url_changed) override; // Favicon download callback. void DidDownloadFavicon( diff --git a/chrome/browser/favicon/favicon_tab_helper_browsertest.cc b/chrome/browser/favicon/favicon_tab_helper_browsertest.cc index 25e2223..8722b5b 100644 --- a/chrome/browser/favicon/favicon_tab_helper_browsertest.cc +++ b/chrome/browser/favicon/favicon_tab_helper_browsertest.cc @@ -6,6 +6,7 @@ #include "base/memory/weak_ptr.h" #include "base/run_loop.h" +#include "base/scoped_observer.h" #include "chrome/app/chrome_command_ids.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/ui/browser.h" @@ -13,6 +14,7 @@ #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/ui_test_utils.h" +#include "components/favicon/core/favicon_driver_observer.h" #include "components/favicon/core/favicon_handler.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" @@ -99,16 +101,19 @@ class FaviconTabHelperPendingTaskChecker { // - The pending navigation. // - FaviconHandler's pending favicon database requests. // - FaviconHandler's pending downloads. -class PendingTaskWaiter : public content::NotificationObserver { +class PendingTaskWaiter : public content::NotificationObserver, + public favicon::FaviconDriverObserver { public: PendingTaskWaiter(content::WebContents* web_contents, FaviconTabHelperPendingTaskChecker* checker) - : checker_(checker), load_stopped_(false), weak_factory_(this) { - registrar_.Add(this, chrome::NOTIFICATION_FAVICON_UPDATED, - content::Source<content::WebContents>(web_contents)); + : checker_(checker), + load_stopped_(false), + scoped_observer_(this), + weak_factory_(this) { registrar_.Add(this, content::NOTIFICATION_LOAD_STOP, content::Source<content::NavigationController>( &web_contents->GetController())); + scoped_observer_.Add(FaviconTabHelper::FromWebContents(web_contents)); } ~PendingTaskWaiter() override {} @@ -129,10 +134,21 @@ class PendingTaskWaiter : public content::NotificationObserver { if (type == content::NOTIFICATION_LOAD_STOP) load_stopped_ = true; + OnNotification(); + } + + // favicon::Favicon + void OnFaviconAvailable(const gfx::Image& image) override {} + void OnFaviconUpdated(favicon::FaviconDriver* favicon_driver, + bool icon_url_changed) override { + OnNotification(); + } + + void OnNotification() { if (!quit_closure_.is_null()) { // We stop waiting based on changes in state to FaviconHandler which occur - // immediately after NOTIFICATION_FAVICON_UPDATED is sent. Post a task to - // check if we can stop waiting. + // immediately after OnFaviconUpdated() is called. Post a task to check if + // we can stop waiting. base::MessageLoopForUI::current()->PostTask( FROM_HERE, base::Bind(&PendingTaskWaiter::EndLoopIfCanStopWaiting, weak_factory_.GetWeakPtr())); @@ -151,6 +167,7 @@ class PendingTaskWaiter : public content::NotificationObserver { bool load_stopped_; base::Closure quit_closure_; content::NotificationRegistrar registrar_; + ScopedObserver<FaviconTabHelper, PendingTaskWaiter> scoped_observer_; base::WeakPtrFactory<PendingTaskWaiter> weak_factory_; DISALLOW_COPY_AND_ASSIGN(PendingTaskWaiter); diff --git a/chrome/browser/prerender/prerender_browsertest.cc b/chrome/browser/prerender/prerender_browsertest.cc index 8f68a2d..f2e31de 100644 --- a/chrome/browser/prerender/prerender_browsertest.cc +++ b/chrome/browser/prerender/prerender_browsertest.cc @@ -8,12 +8,14 @@ #include "base/bind.h" #include "base/command_line.h" #include "base/files/file_path.h" +#include "base/macros.h" #include "base/memory/ref_counted_memory.h" #include "base/memory/scoped_vector.h" #include "base/memory/weak_ptr.h" #include "base/path_service.h" #include "base/prefs/pref_service.h" #include "base/run_loop.h" +#include "base/scoped_observer.h" #include "base/strings/string_util.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" @@ -66,6 +68,7 @@ #include "chrome/test/base/test_switches.h" #include "chrome/test/base/ui_test_utils.h" #include "components/content_settings/core/browser/host_content_settings_map.h" +#include "components/favicon/core/favicon_driver_observer.h" #include "components/variations/entropy_provider.h" #include "components/variations/variations_associated_data.h" #include "content/public/browser/browser_message_filter.h" @@ -131,6 +134,42 @@ namespace prerender { namespace { +class FaviconUpdateWatcher : public favicon::FaviconDriverObserver { + public: + explicit FaviconUpdateWatcher(content::WebContents* web_contents) + : seen_(false), running_(false), scoped_observer_(this) { + scoped_observer_.Add(FaviconTabHelper::FromWebContents(web_contents)); + } + + void Wait() { + if (seen_) + return; + + running_ = true; + message_loop_runner_ = new content::MessageLoopRunner; + message_loop_runner_->Run(); + } + + private: + void OnFaviconAvailable(const gfx::Image& image) override {} + void OnFaviconUpdated(favicon::FaviconDriver* favicon_driver, + bool icon_url_changed) override { + seen_ = true; + if (!running_) + return; + + message_loop_runner_->Quit(); + running_ = false; + } + + bool seen_; + bool running_; + ScopedObserver<FaviconTabHelper, FaviconUpdateWatcher> scoped_observer_; + scoped_refptr<content::MessageLoopRunner> message_loop_runner_; + + DISALLOW_COPY_AND_ASSIGN(FaviconUpdateWatcher); +}; + class MockNetworkChangeNotifierWIFI : public NetworkChangeNotifier { public: ConnectionType GetCurrentConnectionType() const override { @@ -3146,9 +3185,7 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderFavicon) { if (!FaviconTabHelper::FromWebContents( GetActiveWebContents())->FaviconIsValid()) { // If the favicon has not been set yet, wait for it to be. - content::WindowedNotificationObserver favicon_update_watcher( - chrome::NOTIFICATION_FAVICON_UPDATED, - content::Source<WebContents>(GetActiveWebContents())); + FaviconUpdateWatcher favicon_update_watcher(GetActiveWebContents()); favicon_update_watcher.Wait(); } EXPECT_TRUE(FaviconTabHelper::FromWebContents( diff --git a/components/favicon/core/favicon_driver.h b/components/favicon/core/favicon_driver.h index 59fc333..4347ecb 100644 --- a/components/favicon/core/favicon_driver.h +++ b/components/favicon/core/favicon_driver.h @@ -74,6 +74,11 @@ class FaviconDriver { const GURL& icon_url, bool is_active_favicon) = 0; + // Sends notification that the current page favicon has changed. + // |icon_url_changed| is true if the URL of the favicon changed in addition to + // the favicon image. + virtual void NotifyFaviconUpdated(bool icon_url_changed) = 0; + protected: FaviconDriver() {} virtual ~FaviconDriver() {} diff --git a/components/favicon/core/favicon_driver_observer.h b/components/favicon/core/favicon_driver_observer.h index cc134d2..eb3fd48 100644 --- a/components/favicon/core/favicon_driver_observer.h +++ b/components/favicon/core/favicon_driver_observer.h @@ -13,6 +13,8 @@ class Image; namespace favicon { +class FaviconDriver; + // An observer implemented by classes which are interested in event from // FaviconDriver. class FaviconDriverObserver { @@ -24,6 +26,11 @@ class FaviconDriverObserver { // storage. virtual void OnFaviconAvailable(const gfx::Image& image) = 0; + // Called when favicon has changed for the current page. |icon_url_changed| is + // true if the favicon URL has also changed. + virtual void OnFaviconUpdated(FaviconDriver* favicon_driver, + bool icon_url_changed) = 0; + private: DISALLOW_COPY_AND_ASSIGN(FaviconDriverObserver); }; |