Commit 49ad9b78 authored by Mohammad Refaat's avatar Mohammad Refaat Committed by Commit Bot

Stop sending TabChange notification for Favicon updates.

TabStripController used didChangeTab TabModelObserver's method
to monitor favicon changes. This was done through TabModelFaviconDriverObserver
which used to forward to Favicon driver notification through didChangeTab.
So to remove the connection between didChangeTab and favicon notification i
changed TabModelFaviconDriverObserver to be WebStateListFaviconDriverObserver
and it will have a bridge to be used to monitor the favicon driver observer.
Also instead of going through TabModel, WebStateListFaviconObserver will be
monitoring the webStateList given at its constructor and Observing each of its'
webstates favicon drivers. TabStripController conform to the bridge protocol and
creates a WebStateListFaviconObserver giving it the webStateList to monitor its
drivers.



Bug: 911350
Change-Id: I19a8523c368aa06dabc80005ffcf65d78c00e07f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626544
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663090}
parent d5fa4425
...@@ -44,8 +44,6 @@ source_set("tabs_internal") { ...@@ -44,8 +44,6 @@ source_set("tabs_internal") {
"tab_model.mm", "tab_model.mm",
"tab_model_closing_web_state_observer.h", "tab_model_closing_web_state_observer.h",
"tab_model_closing_web_state_observer.mm", "tab_model_closing_web_state_observer.mm",
"tab_model_favicon_driver_observer.h",
"tab_model_favicon_driver_observer.mm",
"tab_model_list.mm", "tab_model_list.mm",
"tab_model_observers.h", "tab_model_observers.h",
"tab_model_observers.mm", "tab_model_observers.mm",
...@@ -150,7 +148,6 @@ source_set("tabs_internal") { ...@@ -150,7 +148,6 @@ source_set("tabs_internal") {
source_set("unit_tests") { source_set("unit_tests") {
testonly = true testonly = true
sources = [ sources = [
"tab_model_favicon_driver_observer_unittest.mm",
"tab_model_list_unittest.mm", "tab_model_list_unittest.mm",
"tab_model_unittest.mm", "tab_model_unittest.mm",
"tab_title_util_unittest.mm", "tab_title_util_unittest.mm",
...@@ -162,7 +159,6 @@ source_set("unit_tests") { ...@@ -162,7 +159,6 @@ source_set("unit_tests") {
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//components/bookmarks/test", "//components/bookmarks/test",
"//components/favicon/ios",
"//components/history/core/browser", "//components/history/core/browser",
"//components/keyed_service/core", "//components/keyed_service/core",
"//components/search_engines", "//components/search_engines",
...@@ -200,7 +196,6 @@ source_set("unit_tests") { ...@@ -200,7 +196,6 @@ source_set("unit_tests") {
"//testing/gtest", "//testing/gtest",
"//third_party/ocmock", "//third_party/ocmock",
"//ui/base", "//ui/base",
"//ui/gfx",
] ]
configs += [ "//build/config/compiler:enable_arc" ] configs += [ "//build/config/compiler:enable_arc" ]
} }
...@@ -41,7 +41,6 @@ ...@@ -41,7 +41,6 @@
#import "ios/chrome/browser/tabs/legacy_tab_helper.h" #import "ios/chrome/browser/tabs/legacy_tab_helper.h"
#import "ios/chrome/browser/tabs/tab.h" #import "ios/chrome/browser/tabs/tab.h"
#import "ios/chrome/browser/tabs/tab_model_closing_web_state_observer.h" #import "ios/chrome/browser/tabs/tab_model_closing_web_state_observer.h"
#import "ios/chrome/browser/tabs/tab_model_favicon_driver_observer.h"
#import "ios/chrome/browser/tabs/tab_model_list.h" #import "ios/chrome/browser/tabs/tab_model_list.h"
#import "ios/chrome/browser/tabs/tab_model_observers.h" #import "ios/chrome/browser/tabs/tab_model_observers.h"
#import "ios/chrome/browser/tabs/tab_model_selected_tab_observer.h" #import "ios/chrome/browser/tabs/tab_model_selected_tab_observer.h"
...@@ -365,9 +364,6 @@ void RecordMainFrameNavigationMetric(web::WebState* web_state) { ...@@ -365,9 +364,6 @@ void RecordMainFrameNavigationMetric(web::WebState* web_state) {
std::make_unique<WebStateListObserverBridge>( std::make_unique<WebStateListObserverBridge>(
tabModelSelectedTabObserver)); tabModelSelectedTabObserver));
_webStateListObservers.push_back(
std::make_unique<TabModelFaviconDriverObserver>(self, _observers));
auto webStateListMetricsObserver = auto webStateListMetricsObserver =
std::make_unique<WebStateListMetricsObserver>(); std::make_unique<WebStateListMetricsObserver>();
_webStateListMetricsObserver = webStateListMetricsObserver.get(); _webStateListMetricsObserver = webStateListMetricsObserver.get();
......
...@@ -45,6 +45,7 @@ ...@@ -45,6 +45,7 @@
#include "ios/chrome/browser/ui/util/ui_util.h" #include "ios/chrome/browser/ui/util/ui_util.h"
#import "ios/chrome/browser/ui/util/uikit_ui_util.h" #import "ios/chrome/browser/ui/util/uikit_ui_util.h"
#import "ios/chrome/browser/web_state_list/web_state_list.h" #import "ios/chrome/browser/web_state_list/web_state_list.h"
#import "ios/chrome/browser/web_state_list/web_state_list_favicon_driver_observer.h"
#import "ios/chrome/browser/web_state_list/web_state_list_observer_bridge.h" #import "ios/chrome/browser/web_state_list/web_state_list_observer_bridge.h"
#include "ios/chrome/grit/ios_strings.h" #include "ios/chrome/grit/ios_strings.h"
#import "ios/web/public/web_state/web_state.h" #import "ios/web/public/web_state/web_state.h"
...@@ -160,6 +161,7 @@ UIColor* BackgroundColor() { ...@@ -160,6 +161,7 @@ UIColor* BackgroundColor() {
TabStripViewLayoutDelegate, TabStripViewLayoutDelegate,
TabViewDelegate, TabViewDelegate,
WebStateListObserving, WebStateListObserving,
WebStateFaviconDriverObserver,
UIGestureRecognizerDelegate, UIGestureRecognizerDelegate,
UIScrollViewDelegate> { UIScrollViewDelegate> {
TabModel* _tabModel; TabModel* _tabModel;
...@@ -237,6 +239,11 @@ UIColor* BackgroundColor() { ...@@ -237,6 +239,11 @@ UIColor* BackgroundColor() {
// Bridges C++ WebStateListObserver methods to this TabStripController. // Bridges C++ WebStateListObserver methods to this TabStripController.
std::unique_ptr<WebStateListObserverBridge> _webStateListObserver; std::unique_ptr<WebStateListObserverBridge> _webStateListObserver;
// Bridges FaviconDriverObservers methods to this TabStripController, and
// maintains a FaviconObserver for each all webstates.
std::unique_ptr<WebStateListFaviconDriverObserver>
_webStateListFaviconObserver;
API_AVAILABLE(ios(11.0)) DropAndNavigateInteraction* _buttonNewTabInteraction; API_AVAILABLE(ios(11.0)) DropAndNavigateInteraction* _buttonNewTabInteraction;
} }
...@@ -399,6 +406,9 @@ UIColor* BackgroundColor() { ...@@ -399,6 +406,9 @@ UIColor* BackgroundColor() {
[_tabModel addObserver:self]; [_tabModel addObserver:self];
_webStateListObserver = std::make_unique<WebStateListObserverBridge>(self); _webStateListObserver = std::make_unique<WebStateListObserverBridge>(self);
_tabModel.webStateList->AddObserver(_webStateListObserver.get()); _tabModel.webStateList->AddObserver(_webStateListObserver.get());
_webStateListFaviconObserver =
std::make_unique<WebStateListFaviconDriverObserver>(
_tabModel.webStateList, self);
_style = style; _style = style;
_dispatcher = dispatcher; _dispatcher = dispatcher;
...@@ -963,15 +973,6 @@ UIColor* BackgroundColor() { ...@@ -963,15 +973,6 @@ UIColor* BackgroundColor() {
NSUInteger index = [self indexForModelIndex:modelIndex]; NSUInteger index = [self indexForModelIndex:modelIndex];
TabView* view = [_tabArray objectAtIndex:index]; TabView* view = [_tabArray objectAtIndex:index];
[view setTitle:tab_util::GetTabTitle(webState)]; [view setTitle:tab_util::GetTabTitle(webState)];
[view setFavicon:nil];
favicon::FaviconDriver* faviconDriver =
favicon::WebFaviconDriver::FromWebState(webState);
if (faviconDriver && faviconDriver->FaviconIsValid()) {
gfx::Image favicon = faviconDriver->GetFavicon();
if (!favicon.IsEmpty())
[view setFavicon:favicon.ToUIImage()];
}
if (webState->IsLoading() && !IsVisibleURLNewTabPage(webState)) if (webState->IsLoading() && !IsVisibleURLNewTabPage(webState))
[view startProgressSpinner]; [view startProgressSpinner];
...@@ -1076,14 +1077,31 @@ UIColor* BackgroundColor() { ...@@ -1076,14 +1077,31 @@ UIColor* BackgroundColor() {
[self updateTabCount]; [self updateTabCount];
} }
// Observer method, WebState replaced in |webStateList|. #pragma mark -
- (void)webStateList:(WebStateList*)webStateList #pragma mark WebStateFaviconDriverObserver
didReplaceWebState:(web::WebState*)oldWebState
withWebState:(web::WebState*)newWebState // Observer method. |webState| got a favicon update.
atIndex:(int)atIndex { - (void)faviconDriver:(favicon::FaviconDriver*)driver
// TabViews do not hold references to their parent Tabs, so it's safe to treat didUpdateFaviconForWebState:(web::WebState*)webState {
// this as a tab change rather than a tab replace. if (!driver)
[self updateTabViewForWebState:newWebState]; return;
int modelIndex = _tabModel.webStateList->GetIndexOfWebState(webState);
if (modelIndex == WebStateList::kInvalidIndex) {
DCHECK(false) << "Received FavIcon update notification for webState that is"
" not in the WebStateList";
return;
}
NSUInteger index = [self indexForModelIndex:modelIndex];
TabView* view = [_tabArray objectAtIndex:index];
[view setFavicon:nil];
if (driver->FaviconIsValid()) {
gfx::Image favicon = driver->GetFavicon();
if (!favicon.IsEmpty())
[view setFavicon:favicon.ToUIImage()];
}
} }
#pragma mark - #pragma mark -
......
...@@ -11,6 +11,8 @@ source_set("web_state_list") { ...@@ -11,6 +11,8 @@ source_set("web_state_list") {
"web_state_list.h", "web_state_list.h",
"web_state_list.mm", "web_state_list.mm",
"web_state_list_delegate.h", "web_state_list_delegate.h",
"web_state_list_favicon_driver_observer.h",
"web_state_list_favicon_driver_observer.mm",
"web_state_list_metrics_observer.h", "web_state_list_metrics_observer.h",
"web_state_list_metrics_observer.mm", "web_state_list_metrics_observer.mm",
"web_state_list_observer.h", "web_state_list_observer.h",
...@@ -26,6 +28,8 @@ source_set("web_state_list") { ...@@ -26,6 +28,8 @@ source_set("web_state_list") {
] ]
deps = [ deps = [
"//base", "//base",
"//components/favicon/core",
"//components/favicon/ios",
"//ios/chrome/browser/sessions:serialisation", "//ios/chrome/browser/sessions:serialisation",
"//ios/web", "//ios/web",
] ]
...@@ -51,6 +55,7 @@ source_set("unit_tests") { ...@@ -51,6 +55,7 @@ source_set("unit_tests") {
sources = [ sources = [
"active_web_state_observation_forwarder_unittest.mm", "active_web_state_observation_forwarder_unittest.mm",
"all_web_state_observation_forwarder_unittest.mm", "all_web_state_observation_forwarder_unittest.mm",
"web_state_list_favicon_driver_observer_unittest.mm",
"web_state_list_order_controller_unittest.mm", "web_state_list_order_controller_unittest.mm",
"web_state_list_serialization_unittest.mm", "web_state_list_serialization_unittest.mm",
"web_state_list_unittest.mm", "web_state_list_unittest.mm",
...@@ -60,11 +65,16 @@ source_set("unit_tests") { ...@@ -60,11 +65,16 @@ source_set("unit_tests") {
":test_support", ":test_support",
":web_state_list", ":web_state_list",
"//base", "//base",
"//components/favicon/ios",
"//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/sessions:serialisation", "//ios/chrome/browser/sessions:serialisation",
"//ios/chrome/browser/web",
"//ios/web", "//ios/web",
"//ios/web/public/test",
"//ios/web/public/test/fakes", "//ios/web/public/test/fakes",
"//net", "//net",
"//testing/gtest", "//testing/gtest",
"//ui/gfx",
"//url", "//url",
] ]
configs += [ "//build/config/compiler:enable_arc" ] configs += [ "//build/config/compiler:enable_arc" ]
......
...@@ -2,28 +2,40 @@ ...@@ -2,28 +2,40 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#ifndef IOS_CHROME_BROWSER_TABS_TAB_MODEL_FAVICON_DRIVER_OBSERVER_H_ #ifndef IOS_CHROME_BROWSER_WEB_STATE_LIST_WEB_STATE_LIST_FAVICON_DRIVER_OBSERVER_H_
#define IOS_CHROME_BROWSER_TABS_TAB_MODEL_FAVICON_DRIVER_OBSERVER_H_ #define IOS_CHROME_BROWSER_WEB_STATE_LIST_WEB_STATE_LIST_FAVICON_DRIVER_OBSERVER_H_
#include <map> #include <map>
#include "base/macros.h" #include "base/macros.h"
#include "base/scoped_observer.h"
#include "components/favicon/core/favicon_driver_observer.h" #include "components/favicon/core/favicon_driver_observer.h"
#import "ios/chrome/browser/web_state_list/web_state_list_observer.h" #import "ios/chrome/browser/web_state_list/web_state_list_observer.h"
@class TabModel; class WebStateList;
@class TabModelObservers;
namespace web {
class WebState;
} // namespace web
// Listen to multiple FaviconDriver for notification that their WebState's @protocol WebStateFaviconDriverObserver
// favicon has changed and forward the notifications to TabModelObservers. // Forward the call from |driver| OnFaviconUpdated method.
- (void)faviconDriver:(favicon::FaviconDriver*)driver
didUpdateFaviconForWebState:(web::WebState*)webState;
@end
// Listen to multiple FaviconDrivers for notification that their WebState's
// favicon has changed and forward the notifications to FaviconDriverObserving.
// The class listen to a WebStateList for the creation/replacement/removal // The class listen to a WebStateList for the creation/replacement/removal
// of WebStates. // of WebStates.
class TabModelFaviconDriverObserver : public favicon::FaviconDriverObserver, class WebStateListFaviconDriverObserver
public WebStateListObserver { : public WebStateListObserver,
public favicon::FaviconDriverObserver {
public: public:
TabModelFaviconDriverObserver(TabModel* tab_model, WebStateListFaviconDriverObserver(WebStateList* web_state_list,
TabModelObservers* observers); id<WebStateFaviconDriverObserver> observer);
~TabModelFaviconDriverObserver() override; ~WebStateListFaviconDriverObserver() override;
// WebStateListObserver implementation: // WebStateListObserver implementation:
void WebStateInsertedAt(WebStateList* web_state_list, void WebStateInsertedAt(WebStateList* web_state_list,
...@@ -46,20 +58,22 @@ class TabModelFaviconDriverObserver : public favicon::FaviconDriverObserver, ...@@ -46,20 +58,22 @@ class TabModelFaviconDriverObserver : public favicon::FaviconDriverObserver,
const gfx::Image& image) override; const gfx::Image& image) override;
private: private:
// The owning TabModel. This will be passed to the TabModelObservers // Observes the FaviconDriver for |web_state| and updates the
// when forwarding notificaton. // |driver_to_web_state_map_|.
__weak TabModel* tab_model_; void AddNewWebState(web::WebState* web_state);
// The TabModelObservers to which the FaviconDriver notification are // The WebStateFaviconDriverObserver to which the FaviconDriver notification
// forwarded. Should not be nil. // are forwarded. Should not be nil.
__weak TabModelObservers* observers_; __weak id<WebStateFaviconDriverObserver> favicon_observer_;
// Maps FaviconDriver to the WebState they are attached to. Used // Maps FaviconDriver to the WebState they are attached to. Used
// to find the WebState that should be passed when forwarding the // to find the WebState that should be passed when forwarding the
// notification to TabModelObservers. // notification to TabModelObservers.
std::map<favicon::FaviconDriver*, web::WebState*> driver_to_web_state_map_; std::map<favicon::FaviconDriver*, web::WebState*> driver_to_web_state_map_;
DISALLOW_COPY_AND_ASSIGN(TabModelFaviconDriverObserver); ScopedObserver<WebStateList, WebStateListObserver> web_state_list_observer_;
DISALLOW_COPY_AND_ASSIGN(WebStateListFaviconDriverObserver);
}; };
#endif // IOS_CHROME_BROWSER_TABS_TAB_MODEL_FAVICON_DRIVER_OBSERVER_H_ #endif // IOS_CHROME_BROWSER_WEB_STATE_LIST_WEB_STATE_LIST_FAVICON_DRIVER_OBSERVER_H_
...@@ -2,23 +2,28 @@ ...@@ -2,23 +2,28 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#import "ios/chrome/browser/tabs/tab_model_favicon_driver_observer.h" #import "ios/chrome/browser/web_state_list/web_state_list_favicon_driver_observer.h"
#include "base/logging.h"
#include "components/favicon/ios/web_favicon_driver.h" #include "components/favicon/ios/web_favicon_driver.h"
#import "ios/chrome/browser/tabs/legacy_tab_helper.h"
#import "ios/chrome/browser/tabs/tab_model_observers.h" #include "base/logging.h"
#include "ios/chrome/browser/web_state_list/web_state_list.h"
#import "ios/web/public/web_state/web_state.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
TabModelFaviconDriverObserver::TabModelFaviconDriverObserver( WebStateListFaviconDriverObserver::WebStateListFaviconDriverObserver(
TabModel* tab_model, WebStateList* web_state_list,
TabModelObservers* observers) id<WebStateFaviconDriverObserver> observer)
: tab_model_(tab_model), observers_(observers) {} : favicon_observer_(observer), web_state_list_observer_(this) {
web_state_list_observer_.Add(web_state_list);
for (int i = 0; i < web_state_list->count(); ++i)
AddNewWebState(web_state_list->GetWebStateAt(i));
}
TabModelFaviconDriverObserver::~TabModelFaviconDriverObserver() { WebStateListFaviconDriverObserver::~WebStateListFaviconDriverObserver() {
if (!driver_to_web_state_map_.empty()) { if (!driver_to_web_state_map_.empty()) {
for (const auto& pair : driver_to_web_state_map_) { for (const auto& pair : driver_to_web_state_map_) {
favicon::FaviconDriver* driver = pair.first; favicon::FaviconDriver* driver = pair.first;
...@@ -28,55 +33,45 @@ TabModelFaviconDriverObserver::~TabModelFaviconDriverObserver() { ...@@ -28,55 +33,45 @@ TabModelFaviconDriverObserver::~TabModelFaviconDriverObserver() {
} }
} }
void TabModelFaviconDriverObserver::WebStateInsertedAt( void WebStateListFaviconDriverObserver::WebStateInsertedAt(
WebStateList* web_state_list, WebStateList* web_state_list,
web::WebState* web_state, web::WebState* web_state,
int index, int index,
bool activating) { bool activating) {
// Forward to WebStateReplacedAt as an insertion can be simulated as AddNewWebState(web_state);
// replacing a null WebState by a real one.
WebStateReplacedAt(web_state_list, nullptr, web_state, index);
} }
void TabModelFaviconDriverObserver::WebStateReplacedAt( void WebStateListFaviconDriverObserver::WebStateReplacedAt(
WebStateList* web_state_list, WebStateList* web_state_list,
web::WebState* old_web_state, web::WebState* old_web_state,
web::WebState* new_web_state, web::WebState* new_web_state,
int index) { int index) {
if (old_web_state) { if (old_web_state) {
favicon::WebFaviconDriver* driver = // Forward to WebStateDetachedAt as this is considered a webState removal.
favicon::WebFaviconDriver::FromWebState(old_web_state); WebStateDetachedAt(web_state_list, old_web_state, index);
if (driver) {
auto iterator = driver_to_web_state_map_.find(driver);
DCHECK(iterator != driver_to_web_state_map_.end());
DCHECK(iterator->second == old_web_state);
driver_to_web_state_map_.erase(iterator);
driver->RemoveObserver(this);
}
} }
if (new_web_state) { if (new_web_state) {
favicon::WebFaviconDriver* driver = AddNewWebState(new_web_state);
favicon::WebFaviconDriver::FromWebState(new_web_state);
if (driver) {
auto iterator = driver_to_web_state_map_.find(driver);
DCHECK(iterator == driver_to_web_state_map_.end());
driver_to_web_state_map_[driver] = new_web_state;
driver->AddObserver(this);
}
} }
} }
void TabModelFaviconDriverObserver::WebStateDetachedAt( void WebStateListFaviconDriverObserver::WebStateDetachedAt(
WebStateList* web_state_list, WebStateList* web_state_list,
web::WebState* web_state, web::WebState* web_state,
int index) { int index) {
// Forward to WebStateReplacedAt as a removal can be simulated as favicon::WebFaviconDriver* driver =
// replacing a real WebState by a null one. favicon::WebFaviconDriver::FromWebState(web_state);
WebStateReplacedAt(web_state_list, web_state, nullptr, index); if (driver) {
auto iterator = driver_to_web_state_map_.find(driver);
DCHECK(iterator != driver_to_web_state_map_.end());
DCHECK(iterator->second == web_state);
driver_to_web_state_map_.erase(iterator);
driver->RemoveObserver(this);
}
} }
void TabModelFaviconDriverObserver::OnFaviconUpdated( void WebStateListFaviconDriverObserver::OnFaviconUpdated(
favicon::FaviconDriver* driver, favicon::FaviconDriver* driver,
favicon::FaviconDriverObserver::NotificationIconType icon_type, favicon::FaviconDriverObserver::NotificationIconType icon_type,
const GURL& icon_url, const GURL& icon_url,
...@@ -85,7 +80,18 @@ void TabModelFaviconDriverObserver::OnFaviconUpdated( ...@@ -85,7 +80,18 @@ void TabModelFaviconDriverObserver::OnFaviconUpdated(
auto iterator = driver_to_web_state_map_.find(driver); auto iterator = driver_to_web_state_map_.find(driver);
DCHECK(iterator != driver_to_web_state_map_.end()); DCHECK(iterator != driver_to_web_state_map_.end());
DCHECK(iterator->second); DCHECK(iterator->second);
[favicon_observer_ faviconDriver:driver
didUpdateFaviconForWebState:iterator->second];
}
[observers_ tabModel:tab_model_ void WebStateListFaviconDriverObserver::AddNewWebState(
didChangeTab:LegacyTabHelper::GetTabForWebState(iterator->second)]; web::WebState* web_state) {
favicon::WebFaviconDriver* driver =
favicon::WebFaviconDriver::FromWebState(web_state);
if (driver) {
auto iterator = driver_to_web_state_map_.find(driver);
DCHECK(iterator == driver_to_web_state_map_.end());
driver_to_web_state_map_[driver] = web_state;
driver->AddObserver(this);
}
} }
...@@ -2,15 +2,11 @@ ...@@ -2,15 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#import "ios/chrome/browser/tabs/tab_model_favicon_driver_observer.h" #import "ios/chrome/browser/web_state_list/web_state_list_favicon_driver_observer.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/scoped_observer.h"
#include "components/favicon/ios/web_favicon_driver.h" #include "components/favicon/ios/web_favicon_driver.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h" #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#import "ios/chrome/browser/tabs/legacy_tab_helper.h"
#import "ios/chrome/browser/tabs/tab_model_observers.h"
#import "ios/chrome/browser/web/tab_id_tab_helper.h"
#import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h" #import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h"
#import "ios/chrome/browser/web_state_list/web_state_list.h" #import "ios/chrome/browser/web_state_list/web_state_list.h"
#import "ios/chrome/browser/web_state_list/web_state_opener.h" #import "ios/chrome/browser/web_state_list/web_state_opener.h"
...@@ -25,38 +21,37 @@ ...@@ -25,38 +21,37 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
@interface TabModelFaviconDriverObserverTestTabModelObserver @interface FakeWebStateFaviconDriverObserver
: NSObject<TabModelObserver> : NSObject <WebStateFaviconDriverObserver>
@property(nonatomic, readonly) BOOL tabModelDidChangeTabCalled; @property(nonatomic, readonly) BOOL didUpdateFaviconCalled;
@end @end
@implementation TabModelFaviconDriverObserverTestTabModelObserver @implementation FakeWebStateFaviconDriverObserver
@synthesize tabModelDidChangeTabCalled = _tabModelDidChangeTabCalled; @synthesize didUpdateFaviconCalled = _didUpdateFaviconCalled;
- (void)tabModel:(TabModel*)model didChangeTab:(Tab*)tab { - (void)faviconDriver:(favicon::FaviconDriver*)driver
_tabModelDidChangeTabCalled = YES; didUpdateFaviconForWebState:(web::WebState*)webState {
_didUpdateFaviconCalled = YES;
} }
@end @end
class TabModelFaviconDriverObserverTest : public PlatformTest { class WebStateListFaviconDriverObserverTest : public PlatformTest {
public: public:
TabModelFaviconDriverObserverTest(); WebStateListFaviconDriverObserverTest();
~TabModelFaviconDriverObserverTest() override = default; ~WebStateListFaviconDriverObserverTest() override = default;
void TearDown() override; favicon::FaviconDriver* CreateAndInsertWebState();
favicon::FaviconDriver* CreateAndInsertTab(); WebStateListFaviconDriverObserver* web_state_list_favicon_driver_observer() {
return &web_state_list_favicon_driver_observer_;
TabModelFaviconDriverObserver* tab_model_favicon_driver_observer() {
return &tab_model_favicon_driver_observer_;
} }
TabModelFaviconDriverObserverTestTabModelObserver* tab_model_observer() { FakeWebStateFaviconDriverObserver* favicon_observer() {
return tab_model_observer_; return favicon_observer_;
} }
private: private:
...@@ -64,52 +59,24 @@ class TabModelFaviconDriverObserverTest : public PlatformTest { ...@@ -64,52 +59,24 @@ class TabModelFaviconDriverObserverTest : public PlatformTest {
std::unique_ptr<ios::ChromeBrowserState> browser_state_; std::unique_ptr<ios::ChromeBrowserState> browser_state_;
FakeWebStateListDelegate web_state_list_delegate_; FakeWebStateListDelegate web_state_list_delegate_;
WebStateList web_state_list_; WebStateList web_state_list_;
FakeWebStateFaviconDriverObserver* favicon_observer_;
WebStateListFaviconDriverObserver web_state_list_favicon_driver_observer_;
TabModelObservers* tab_model_observers_; DISALLOW_COPY_AND_ASSIGN(WebStateListFaviconDriverObserverTest);
TabModelFaviconDriverObserver tab_model_favicon_driver_observer_;
ScopedObserver<WebStateList, WebStateListObserver>
scoped_web_state_list_observer_;
TabModelFaviconDriverObserverTestTabModelObserver* tab_model_observer_;
DISALLOW_COPY_AND_ASSIGN(TabModelFaviconDriverObserverTest);
}; };
TabModelFaviconDriverObserverTest::TabModelFaviconDriverObserverTest() WebStateListFaviconDriverObserverTest::WebStateListFaviconDriverObserverTest()
: browser_state_(TestChromeBrowserState::Builder().Build()), : browser_state_(TestChromeBrowserState::Builder().Build()),
web_state_list_(&web_state_list_delegate_), web_state_list_(&web_state_list_delegate_),
tab_model_observers_([TabModelObservers observers]), favicon_observer_([[FakeWebStateFaviconDriverObserver alloc] init]),
tab_model_favicon_driver_observer_(nil, tab_model_observers_),
scoped_web_state_list_observer_(&tab_model_favicon_driver_observer_) {
scoped_web_state_list_observer_.Add(&web_state_list_);
tab_model_observer_ =
[[TabModelFaviconDriverObserverTestTabModelObserver alloc] init];
[tab_model_observers_ addObserver:tab_model_observer_];
}
void TabModelFaviconDriverObserverTest::TearDown() { web_state_list_favicon_driver_observer_(&web_state_list_,
// Clear pointer to Objective-C instances to ensure they are released favicon_observer_) {}
// when the PlatformTest autoreleasepool is cleared.
[tab_model_observers_ removeObserver:tab_model_observer_];
tab_model_observer_ = nil;
// It is safe to drop the last strong reference to TabModelObservers here
// because TabModelFaviconDriverObserver only has a weak reference to it.
tab_model_observers_ = nil;
PlatformTest::TearDown();
}
favicon::FaviconDriver* favicon::FaviconDriver*
TabModelFaviconDriverObserverTest::CreateAndInsertTab() { WebStateListFaviconDriverObserverTest::CreateAndInsertWebState() {
std::unique_ptr<web::WebState> web_state = std::unique_ptr<web::WebState> web_state =
web::WebState::Create(web::WebState::CreateParams(browser_state_.get())); web::WebState::Create(web::WebState::CreateParams(browser_state_.get()));
// TODO(crbug.com/783776): once the API has been changed to use WebState
// instead of Tab, then we can remove the creation of the LegacyTabHelper
// and TabIdTabHelper.
TabIdTabHelper::CreateForWebState(web_state.get());
LegacyTabHelper::CreateForWebState(web_state.get());
favicon::WebFaviconDriver::CreateForWebState(web_state.get(), favicon::WebFaviconDriver::CreateForWebState(web_state.get(),
/*favicon_service=*/nullptr); /*favicon_service=*/nullptr);
...@@ -123,14 +90,16 @@ TabModelFaviconDriverObserverTest::CreateAndInsertTab() { ...@@ -123,14 +90,16 @@ TabModelFaviconDriverObserverTest::CreateAndInsertTab() {
return favicon_driver; return favicon_driver;
} }
TEST_F(TabModelFaviconDriverObserverTest, OnFaviconUpdated) { // Tests that calls to OnFaviconUpdated are forwarded correctly to
ASSERT_FALSE([tab_model_observer() tabModelDidChangeTabCalled]); // WebStateFaviconDriverObserver.
TEST_F(WebStateListFaviconDriverObserverTest, OnFaviconUpdated) {
ASSERT_FALSE([favicon_observer() didUpdateFaviconCalled]);
favicon::FaviconDriver* favicon_driver = CreateAndInsertTab(); favicon::FaviconDriver* favicon_driver = CreateAndInsertWebState();
EXPECT_FALSE([tab_model_observer() tabModelDidChangeTabCalled]); ASSERT_FALSE([favicon_observer() didUpdateFaviconCalled]);
tab_model_favicon_driver_observer()->OnFaviconUpdated( web_state_list_favicon_driver_observer()->OnFaviconUpdated(
favicon_driver, favicon::FaviconDriverObserver::TOUCH_LARGEST, GURL(), favicon_driver, favicon::FaviconDriverObserver::TOUCH_LARGEST, GURL(),
true, gfx::Image()); true, gfx::Image());
EXPECT_TRUE([tab_model_observer() tabModelDidChangeTabCalled]); ASSERT_TRUE([favicon_observer() didUpdateFaviconCalled]);
} }
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