Convert some of TabModel logic to WebStateListObservers.

ios/chrome/browser/tabs/tab_model.mm

 // Copyright 2012 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.
 
 #import "ios/chrome/browser/tabs/tab_model.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 13 matching lines @@
 #import "ios/chrome/browser/chrome_url_util.h"
 #import "ios/chrome/browser/metrics/tab_usage_recorder.h"
 #include "ios/chrome/browser/sessions/ios_chrome_tab_restore_service_factory.h"
 #import "ios/chrome/browser/sessions/session_service.h"
 #import "ios/chrome/browser/sessions/session_window.h"
 #import "ios/chrome/browser/snapshots/snapshot_cache.h"
 #include "ios/chrome/browser/tab_parenting_global_observer.h"
 #import "ios/chrome/browser/tabs/legacy_tab_helper.h"
 #import "ios/chrome/browser/tabs/tab.h"
 #import "ios/chrome/browser/tabs/tab_model_list.h"
+#import "ios/chrome/browser/tabs/tab_model_metrics_observer.h"
 #import "ios/chrome/browser/tabs/tab_model_observers.h"
 #import "ios/chrome/browser/tabs/tab_model_order_controller.h"
 #import "ios/chrome/browser/tabs/tab_model_synced_window_delegate.h"
+#import "ios/chrome/browser/tabs/tab_model_tab_model_observers_bridge.h"
+#import "ios/chrome/browser/tabs/tab_model_tab_parenting_observer.h"
 #import "ios/chrome/browser/xcallback_parameters.h"
 #import "ios/shared/chrome/browser/tabs/web_state_list.h"
 #import "ios/shared/chrome/browser/tabs/web_state_list_fast_enumeration_helper.h"
 #import "ios/shared/chrome/browser/tabs/web_state_list_observer.h"
 #import "ios/web/navigation/crw_session_certificate_policy_manager.h"
 #import "ios/web/navigation/crw_session_controller.h"
 #include "ios/web/public/browser_state.h"
 #include "ios/web/public/certificate_policy_cache.h"
 #include "ios/web/public/navigation_item.h"
 #import "ios/web/public/navigation_manager.h"
@@ skipping 74 matching lines @@
 
   // Helper providing NSFastEnumeration implementation over the WebStateList.
   base::scoped_nsobject<WebStateListFastEnumerationHelper>
       _fastEnumerationHelper;
 
   // Used to keep the Tabs alive while the corresponding WebStates are stored
   // in the WebStateList (as Tabs currently owns their WebState). Remove once
   // WebState owns the associated Tab.
   base::scoped_nsobject<NSMutableSet<Tab*>> _tabs;
 
+  // WebStateListObserver bridges to react to modifications of the model (may
+  // send notification, translate and forward events, update metrics, ...).
+  std::vector<std::unique_ptr<WebStateListObserver>> _observerBridges;
+
   // Maintains policy for where new tabs go and the selection when a tab
   // is removed.
   base::scoped_nsobject<TabModelOrderController> _orderController;
   // The delegate for sync.
   std::unique_ptr<TabModelSyncedWindowDelegate> _syncedWindowDelegate;
   // Currently selected tab. May be nil.
   base::WeakNSObject<Tab> _currentTab;
 
   // Counters for metrics.
   int _openedTabCount;
@@ skipping 66 matching lines @@
 @synthesize webUsageEnabled = webUsageEnabled_;
 
 #pragma mark - Overriden
 
 - (void)dealloc {
   DCHECK([_observers empty]);
   // browserStateDestroyed should always have been called before destruction.
   DCHECK(!_browserState);
 
   [[NSNotificationCenter defaultCenter] removeObserver:self];
+
+  // Unregister all listeners before closing all the tabs.
+  for (const auto& observerBridge : _observerBridges)
+    _webStateList.RemoveObserver(observerBridge.get());
+  _observerBridges.clear();
+
   // Make sure the tabs do clean after themselves. It is important for
   // removeObserver: to be called first otherwise a lot of unecessary work will
   // happen on -closeAllTabs.
   [self closeAllTabs];
 
   _clearPoliciesTaskTracker.TryCancelAll();
 
   [super dealloc];
 }
 
@@ skipping 39 matching lines @@
                        sessionService:(SessionServiceIOS*)service
                          browserState:(ios::ChromeBrowserState*)browserState {
   if ((self = [super init])) {
     _tabs.reset([[NSMutableSet alloc] init]);
     _observers.reset([[TabModelObservers observers] retain]);
 
     _fastEnumerationHelper.reset([[WebStateListFastEnumerationHelper alloc]
         initWithWebStateList:&_webStateList
                 proxyFactory:self]);
 
+    _observerBridges.push_back(base::MakeUnique<WebStateListObserverBridge>(
+        [[TabModelTabParentingObserver alloc] init]));
+    _observerBridges.push_back(base::MakeUnique<WebStateListObserverBridge>(
+        [[TabModelTabModelObserversBridge alloc]
+             initWithTabModel:self
+            tabModelObservers:_observers.get()]));
+    _observerBridges.push_back(base::MakeUnique<WebStateListObserverBridge>(
+        [[TabModelMetricsObserver alloc] init]));
+    for (const auto& observerBridge : _observerBridges)
+      _webStateList.AddObserver(observerBridge.get());
+
     _browserState = browserState;
     DCHECK(_browserState);
 
     // There must be a valid session service defined to consume session windows.
     DCHECK(service);
     _sessionService.reset([service retain]);
 
     // Normal browser states are the only ones to get tab restore. Tab sync
     // handles incognito browser states by filtering on profile, so it's
     // important to the backend code to always have a sync window delegate.
@@ skipping 251 matching lines @@
   base::scoped_nsobject<Tab> tab(
       [[Tab alloc] initWithWebState:std::move(webState) model:self]);
   [tab webController].webUsageEnabled = webUsageEnabled_;
   [self insertTab:tab atIndex:index];
   return tab;
 }
 
 - (void)insertTab:(Tab*)tab atIndex:(NSUInteger)index {
   DCHECK(tab);
   DCHECK(![_tabs containsObject:tab]);
-  [tab fetchFavicon];

[marq (ping after 24h), 2017/02/13 17:06:54]

Any idea why [tab setParentTabModel:self] isn't called in this method? I was
wondering if we could just move the -fetchFavicon calls into there, where at
least they would be contained in the "right" place, but it looks like the
current implementation is asymmetric and only calls -setParentTabModel in
-replaceTab:withTab:

[sdefresne, 2017/02/13 17:13:39]

Yes, this is because this method is only ever called from one of TabModel method
that pass "self" as TabModel to the Tab initialiser. Tab -setParentTabModel:
DCHECK that parentTabModel_ ivar is nil or that tabModel is nil (to allow
resetting it before deallocation). The is no way to get the parentTabModel from
Tab public API, so it is not possible to see if it is correct and only
conditionally calling -setParentTabModel:.

I was considering changing this, but I think it should be done as a separate CL
(after introducing a protocol to break the dependency cycle between Tab and
TabModel).

 
   [_tabs addObject:tab];
   _webStateList.InsertWebState(index, tab.webState);
-  TabParentingGlobalObserver::GetInstance()->OnTabParented(tab.webState);
-  [_observers tabModel:self didInsertTab:tab atIndex:index inForeground:NO];
-  [_observers tabModelDidChangeTabCount:self];
 
-  base::RecordAction(base::UserMetricsAction("MobileNewTabOpened"));
   // Persist the session due to a new tab being inserted. If this is a
   // background tab (will not become active), saving now will capture the
   // state properly. If it does eventually become active, another save will
   // be triggered to properly capture the end result.
   [self saveSessionImmediately:NO];
 
   ++_newTabCount;
 }
 
 - (void)moveTab:(Tab*)tab toIndex:(NSUInteger)toIndex {
   size_t fromIndex = _webStateList.GetIndexOfWebState(tab.webState);
   _webStateList.MoveWebStateAt(fromIndex, toIndex);
-  [_observers tabModel:self didMoveTab:tab fromIndex:fromIndex toIndex:toIndex];
 }
 
 - (void)replaceTab:(Tab*)oldTab withTab:(Tab*)newTab {
   NSUInteger index = [self indexOfTab:oldTab];
   DCHECK_NE(NSNotFound, static_cast<NSInteger>(index));
+
   DCHECK([_tabs containsObject:oldTab]);
   DCHECK(![_tabs containsObject:newTab]);
 
   base::scoped_nsobject<Tab> tabSaver([oldTab retain]);
-  [newTab fetchFavicon];
   [_tabs removeObject:oldTab];
   [_tabs addObject:newTab];
   [newTab setParentTabModel:self];
 
   _webStateList.ReplaceWebStateAt(index, newTab.webState);
-  TabParentingGlobalObserver::GetInstance()->OnTabParented(newTab.webState);
-  [_observers tabModel:self didReplaceTab:oldTab withTab:newTab atIndex:index];
 
   if (self.currentTab == oldTab)
     [self changeSelectedTabFrom:nil to:newTab persistState:NO];
 
   [oldTab setParentTabModel:nil];
   [oldTab close];
-
-  // Record a tab clobber, since swapping tabs bypasses the tab code that would
-  // normally log clobbers.
-  base::RecordAction(base::UserMetricsAction("MobileTabClobbered"));
 }
 
 - (void)closeTabAtIndex:(NSUInteger)index {
   DCHECK(index < _webStateList.count());
   [self closeTab:[self tabAtIndex:index]];
 }
 
 - (void)closeTab:(Tab*)tab {
   // Ensure the tab stays alive long enough for us to send out the
   // notice of its destruction to the delegate.
@@ skipping 135 matching lines @@
   // notification. This avoids various parts of the code getting confused
   // when the current tab isn't in the tab model.
   Tab* savedCurrentTab = _currentTab;
   if (closedTab == _currentTab)
     _currentTab.reset(nil);
 
   DCHECK([_tabs containsObject:closedTab]);
   [_tabs removeObject:closedTab];
 
   _webStateList.RemoveWebStateAt(closedTabIndex);
-  [_observers tabModel:self didRemoveTab:closedTab atIndex:closedTabIndex];
-  [_observers tabModelDidChangeTabCount:self];
 
   // Current tab has closed, update the selected tab and swap in its
   // contents. There is nothing to do if a non-selected tab is closed as
   // the selection isn't index-based, therefore it hasn't changed.
   // -changeSelectedTabFrom: will persist the state change, so only do it
   // if the selection isn't changing.
   if (closedTab == savedCurrentTab) {
     [self changeSelectedTabFrom:closedTab to:newSelection persistState:NO];
   } else {
     [self saveSessionImmediately:NO];
   }
-  base::RecordAction(base::UserMetricsAction("MobileTabClosed"));
   ++_closedTabCount;
 }
 
 - (void)navigationCommittedInTab:(Tab*)tab {
   if (self.offTheRecord)
     return;
   if (![tab navigationManager])
     return;
 
   // See if the navigation was within a page; if so ignore it.
@@ skipping 329 matching lines @@
   web::NavigationManager::WebLoadParams params(URL);
   params.referrer = referrer;
   params.transition_type = ui::PAGE_TRANSITION_TYPED;
   [[tab webController] loadWithParams:params];
   [tab webController].webUsageEnabled = webUsageEnabled_;
   [self insertTab:tab atIndex:index];
   return tab;
 }
 
 @end