Reduce the use of tabModel & Tab inside SideSwipeController

Tab and tab model are heavily used inside this class. Use
WebStateListObserver instead of tabModel observer for ActiveTabChange.

Bug: 930816, 931852
Change-Id: Ib8cd6ded93077986439030631e42bea0590a4aec
Reviewed-on: https://chromium-review.googlesource.com/c/1480826
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Rohit Rao <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634992}

ios/chrome/browser/ui/side_swipe/BUILD.gn

@@ -58,6 +58,7 @@ source_set("unit_tests") {
     "//base/test:test_support",
     "//ios/chrome/browser",
     "//ios/chrome/browser/browser_state:test_support",
+    "//ios/chrome/browser/web_state_list",
     "//ios/web/public/test",
     "//testing/gtest",
     "//third_party/ocmock",

ios/chrome/browser/ui/side_swipe/side_swipe_controller.mm

@@ -30,6 +30,7 @@
 #import "ios/chrome/browser/web/tab_id_tab_helper.h"
 #import "ios/chrome/browser/web/web_navigation_util.h"
 #import "ios/chrome/browser/web_state_list/web_state_list.h"
+#import "ios/chrome/browser/web_state_list/web_state_list_observer_bridge.h"
 #import "ios/web/public/navigation_item.h"
 #import "ios/web/public/web_client.h"
 #import "ios/web/public/web_state/web_state_observer_bridge.h"
@@ -57,9 +58,10 @@ const CGFloat kIpadTabSwipeDistance = 100;
 const NSUInteger kIpadGreySwipeTabCount = 8;
 }
 
-@interface SideSwipeController ()<CRWWebStateObserver,
-                                  TabModelObserver,
-                                  UIGestureRecognizerDelegate> {
+@interface SideSwipeController () <CRWWebStateObserver,
+                                   TabModelObserver,
+                                   UIGestureRecognizerDelegate,
+                                   WebStateListObserving> {
  @private
 
   __weak TabModel* model_;
@@ -87,6 +89,9 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
   // Bridge to observe the web state from Objective-C.
   std::unique_ptr<web::WebStateObserverBridge> webStateObserverBridge_;
 
+  // Bridge to observe the WebStateList from Objective-C.
+  std::unique_ptr<WebStateListObserverBridge> webStateListObserver_;
+
   // Scoped observer used to track registration of the WebStateObserverBridge.
   std::unique_ptr<ScopedObserver<web::WebState, web::WebStateObserver>>
       scopedWebStateObserver_;
@@ -110,6 +115,8 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
 @property(nonatomic, assign) BOOL leadingEdgeNavigationEnabled;
 // Whether to allow navigating from the trailing edge.
 @property(nonatomic, assign) BOOL trailingEdgeNavigationEnabled;
+// The WebStateList that is being observed by this controller.
+@property(nonatomic, assign, readonly) WebStateList* webStateList;
 
 // Load grey snapshots for the next |kIpadGreySwipeTabCount| tabs in
 // |direction|.
@@ -139,6 +146,7 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
     _secondaryToolbarSnapshotProvider;
 @synthesize snapshotDelegate = snapshotDelegate_;
 @synthesize tabStripDelegate = tabStripDelegate_;
+@synthesize webStateList = webStateList_;
 
 - (id)initWithTabModel:(TabModel*)model
           browserState:(ios::ChromeBrowserState*)browserState {
@@ -147,6 +155,9 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
   if (self) {
     model_ = model;
     [model_ addObserver:self];
+    webStateList_ = model_.webStateList;
+    webStateListObserver_ = std::make_unique<WebStateListObserverBridge>(self);
+    webStateList_->AddObserver(webStateListObserver_.get());
     webStateObserverBridge_ =
         std::make_unique<web::WebStateObserverBridge>(self);
     scopedWebStateObserver_ =
@@ -159,6 +170,7 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
 }
 
 - (void)dealloc {
+  webStateList_->RemoveObserver(webStateListObserver_.get());
   [model_ removeObserver:self];
 
   scopedWebStateObserver_.reset();
@@ -337,7 +349,7 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
     fullscreenDisabler_ = std::make_unique<ScopedFullscreenDisabler>(
         FullscreenControllerFactory::GetInstance()->GetForBrowserState(
             browserState_));
-    SnapshotTabHelper::FromWebState([model_ currentTab].webState)
+    SnapshotTabHelper::FromWebState(webStateList_->GetActiveWebState())
         ->UpdateSnapshotWithCallback(nil);
     [[NSNotificationCenter defaultCenter]
         postNotificationName:kSideSwipeWillStartNotification
@@ -393,7 +405,7 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
           ->CancelPlaceholderForNextNavigation();
       [model_ setCurrentTab:tab];
     }
-    PagePlaceholderTabHelper::FromWebState([model_ currentTab].webState)
+    PagePlaceholderTabHelper::FromWebState(webStateList_->GetActiveWebState())
         ->CancelPlaceholderForNextNavigation();
 
     // Redisplay the view if it was in overlay preview mode.
@@ -410,10 +422,9 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
 }
 
 - (BOOL)canNavigate:(BOOL)goBack {
-  WebStateList* webStateList = model_.webStateList;
-  if (!webStateList || !webStateList->GetActiveWebState())
+  if (!webStateList_ || !webStateList_->GetActiveWebState())
     return NO;
-  web::WebState* webState = webStateList->GetActiveWebState();
+  web::WebState* webState = webStateList_->GetActiveWebState();
   if (goBack && webState->GetNavigationManager()->CanGoBack()) {
     return YES;
   }
@@ -503,7 +514,8 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
 // Show horizontal swipe stack view for iPhone.
 - (void)handleiPhoneTabSwipe:(SideSwipeGestureRecognizer*)gesture {
   if (gesture.state == UIGestureRecognizerStateBegan) {
-    Tab* currentTab = [model_ currentTab];
+    DCHECK(webStateList_);
+    web::WebState* currentWebState = webStateList_->GetActiveWebState();
 
     inSwipe_ = YES;
 
@@ -513,11 +525,10 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
     // TODO(crbug.com/904992): Do not use SnapshotGeneratorDelegate from
     // SideSwipeController.
     CGFloat headerHeight = 0;
-    if (currentTab.webState) {
-      headerHeight =
-          [self.snapshotDelegate snapshotGenerator:nil
-                     snapshotEdgeInsetsForWebState:currentTab.webState]
-              .top;
+    if (currentWebState) {
+      headerHeight = [self.snapshotDelegate snapshotGenerator:nil
+                                snapshotEdgeInsetsForWebState:currentWebState]
+                         .top;
     }
 
     if (tabSideSwipeView_) {
@@ -539,8 +550,8 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
     }
 
     // Ensure that there's an up-to-date snapshot of the current tab.
-    if (currentTab.webState) {
-      SnapshotTabHelper::FromWebState(currentTab.webState)
+    if (currentWebState) {
+      SnapshotTabHelper::FromWebState(currentWebState)
           ->UpdateSnapshotWithCallback(nil);
     }
 
@@ -649,20 +660,23 @@ const NSUInteger kIpadGreySwipeTabCount = 8;
 
 #pragma mark - TabModelObserver Methods
 
-- (void)tabModel:(TabModel*)model
-    didChangeActiveTab:(Tab*)newTab
-           previousTab:(Tab*)previousTab
-               atIndex:(NSUInteger)index {
+- (void)tabModel:(TabModel*)model didChangeTab:(Tab*)tab {
+  [self updateNavigationEdgeSwipeForWebState:tab.webState];
+}
+
+#pragma mark - WebStateListObserving Methods
+
+- (void)webStateList:(WebStateList*)webStateList
+    didChangeActiveWebState:(web::WebState*)newWebState
+                oldWebState:(web::WebState*)oldWebState
+                    atIndex:(int)atIndex
+                     reason:(int)reason {
   // Toggling the gesture's enabled state off and on will effectively cancel
   // the gesture recognizer.
   [swipeGestureRecognizer_ setEnabled:NO];
   [swipeGestureRecognizer_ setEnabled:YES];
 
-  [self updateNavigationEdgeSwipeForWebState:newTab.webState];
-}
-
-- (void)tabModel:(TabModel*)model didChangeTab:(Tab*)tab {
-  [self updateNavigationEdgeSwipeForWebState:tab.webState];
+  [self updateNavigationEdgeSwipeForWebState:newWebState];
 }
 
 @end

ios/chrome/browser/ui/side_swipe/side_swipe_controller_unittest.mm

@@ -6,6 +6,9 @@
 #include "base/test/scoped_feature_list.h"
 #include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
 #include "ios/chrome/browser/chrome_url_constants.h"
+#import "ios/chrome/browser/web_state_list/web_state_list.h"
+#import "ios/chrome/browser/web_state_list/web_state_list_delegate.h"
+#import "ios/chrome/browser/web_state_list/web_state_opener.h"
 #include "ios/web/public/features.h"
 #import "ios/web/public/navigation_item.h"
 #import "ios/web/public/test/fakes/test_navigation_manager.h"
@@ -28,11 +31,27 @@
 
 namespace {
 
+class TestWebStateListDelegate : public WebStateListDelegate {
+  void WillAddWebState(web::WebState* web_state) override {}
+  void WebStateDetached(web::WebState* web_state) override {}
+};
+
 class SideSwipeControllerTest : public PlatformTest {
  public:
   void SetUp() override {
     // Create a mock for the TabModel that owns the object under test.
     tab_model_ = [OCMockObject niceMockForClass:[TabModel class]];
+    std::unique_ptr<web::TestWebState> original_web_state(
+        std::make_unique<web::TestWebState>());
+
+    web_state_list_ = std::make_unique<WebStateList>(&web_state_list_delegate_);
+    web_state_list_->InsertWebState(0, std::move(original_web_state),
+                                    WebStateList::INSERT_NO_FLAGS,
+                                    WebStateOpener());
+
+    WebStateList* web_state_list = web_state_list_.get();
+    [[[tab_model_ stub] andReturnValue:OCMOCK_VALUE(web_state_list)]
+        webStateList];
 
     TestChromeBrowserState::Builder builder;
     browser_state_ = builder.Build();
@@ -48,8 +67,11 @@ class SideSwipeControllerTest : public PlatformTest {
 
   web::TestWebThreadBundle thread_bundle_;
   std::unique_ptr<TestChromeBrowserState> browser_state_;
+  TestWebStateListDelegate web_state_list_delegate_;
+  std::unique_ptr<WebStateList> web_state_list_;
+
   UIView* view_;
-  TabModel* tab_model_;
+  id tab_model_;
   SideSwipeController* side_swipe_controller_;
   base::test::ScopedFeatureList feature_list_;
 };