[iOS] Decouple completion handler from new tab creation in BVC.

This CL decouples the addition of a tab completion block from the other
tab opening code in the BVC. This will allow the tab opening logic
(which is just tab model manipulation) to be moved out of the BVC.

Bug: 903338
Change-Id: Ibc7ba411c2cbec58dd11ac7cb65e2b9284eb811c
Reviewed-on: https://chromium-review.googlesource.com/c/1323558
Commit-Queue: Mark Cogan <marq@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606825}

ios/chrome/app/application_delegate/url_opener_unittest.mm

@@ -77,11 +77,11 @@ typedef Tab* (^mock_gurl_nsuinteger_pagetransition)(const GURL&,
 
 - (Tab*)addSelectedTabWithURL:(const GURL&)url
                       atIndex:(NSUInteger)position
-                   transition:(ui::PageTransition)transition
-           tabAddedCompletion:(ProceduralBlock)completion;
+                   transition:(ui::PageTransition)transition;
 - (void)expectNewForegroundTab;
 - (void)setActive:(BOOL)active;
 - (TabModel*)tabModel;
+- (void)appendTabAddedCompletion:(ProceduralBlock)completion;
 - (void)browserStateDestroyed;
 - (void)shutdown;
 @end
@@ -96,12 +96,10 @@ typedef Tab* (^mock_gurl_nsuinteger_pagetransition)(const GURL&,
 
 - (Tab*)addSelectedTabWithURL:(const GURL&)url
                       atIndex:(NSUInteger)position
-                   transition:(ui::PageTransition)transition
-           tabAddedCompletion:(ProceduralBlock)completion {
+                   transition:(ui::PageTransition)transition {
   self.tabURL = url;
   self.position = position;
   self.transition = transition;
-  self.foregroundTabWasAddedCompletionBlock = completion;
   return nil;
 }
 
@@ -127,6 +125,10 @@ typedef Tab* (^mock_gurl_nsuinteger_pagetransition)(const GURL&,
   return nil;
 }
 
+- (void)appendTabAddedCompletion:(ProceduralBlock)completion {
+  self.foregroundTabWasAddedCompletionBlock = completion;
+}
+
 - (void)browserStateDestroyed {
   // no-op
 }

ios/chrome/app/main_controller.mm

@@ -2229,10 +2229,10 @@ enum class ShowTabSwitcherSnapshotResult {
   Tab* currentTabInTargetBVC = [[targetBVC tabModel] currentTab];
   if (!(currentTabInTargetBVC.webState &&
         IsURLNtp(currentTabInTargetBVC.webState->GetVisibleURL()))) {
+    [targetBVC appendTabAddedCompletion:tabOpenedCompletion];
     return [targetBVC addSelectedTabWithURL:URL
                                     atIndex:NSNotFound
-                                 transition:transition
-                         tabAddedCompletion:tabOpenedCompletion];
+                                 transition:transition];
   }
 
   Tab* newTab = currentTabInTargetBVC;
@@ -2325,10 +2325,10 @@ enum class ShowTabSwitcherSnapshotResult {
           targetMode == ApplicationMode::NORMAL
               ? TabSwitcherDismissalMode::NORMAL
               : TabSwitcherDismissalMode::INCOGNITO;
+      [targetBVC appendTabAddedCompletion:tabOpenedCompletion];
       tab = [targetBVC addSelectedTabWithURL:url
                                      atIndex:tabIndex
-                                  transition:transition
-                          tabAddedCompletion:tabOpenedCompletion];
+                                  transition:transition];
     } else {
       // Voice search, QRScanner and the omnibox are presented by the BVC.
       // They must be started after the BVC view is added in the hierarchy.

ios/chrome/browser/ui/browser_view_controller.h

@@ -112,11 +112,6 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint
 - (void)openNewTabFromOriginPoint:(CGPoint)originPoint
                      focusOmnibox:(BOOL)focusOmnibox;
 
-// Add a new tab with the given url, appends it to the end of the model,
-// and makes it the selected tab. The selected tab is returned.
-- (Tab*)addSelectedTabWithURL:(const GURL&)url
-                   transition:(ui::PageTransition)transition;
-
 // Add a new tab with the given url, at the given |position|,
 // and makes it the selected tab. The selected tab is returned.
 // If |position| == NSNotFound the tab will be added at the end of the stack.
@@ -124,14 +119,10 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint
                       atIndex:(NSUInteger)position
                    transition:(ui::PageTransition)transition;
 
-// Add a new tab with the given url, at the given |position|,
-// and makes it the selected tab. The selected tab is returned.
-// If |position| == NSNotFound the tab will be added at the end of the stack.
-// |tabAddedCompletion| is called after the tab is added (if not nil).
-- (Tab*)addSelectedTabWithURL:(const GURL&)url
-                      atIndex:(NSUInteger)position
-                   transition:(ui::PageTransition)transition
-           tabAddedCompletion:(ProceduralBlock)tabAddedCompletion;
+// Adds |tabAddedCompletion| to the completion block (if any) that will be run
+// the next time a tab is added to the TabModel this object was initialized
+// with.
+- (void)appendTabAddedCompletion:(ProceduralBlock)tabAddedCompletion;
 
 // Informs the BVC that a new foreground tab is about to be opened. This is
 // intended to be called before setWebUsageSuspended:NO in cases where a new tab

ios/chrome/browser/ui/browser_view_controller.mm

@@ -835,8 +835,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
 - (Tab*)addSelectedTabWithURL:(const GURL&)url
                      postData:(TemplateURLRef::PostContent*)postData
                       atIndex:(NSUInteger)position
-                   transition:(ui::PageTransition)transition
-           tabAddedCompletion:(ProceduralBlock)tabAddedCompletion;
+                   transition:(ui::PageTransition)transition;
 // Whether the given tab's URL is an application specific URL.
 - (BOOL)isTabNativePage:(Tab*)tab;
 // Returns the view to use when animating a page in or out, positioning it to
@@ -1439,34 +1438,32 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint {
         ->UpdateSnapshot(/*with_overlays=*/true, /*visible_frame_only=*/true);
   }
   [self addSelectedTabWithURL:GURL(kChromeUINewTabURL)
-                   transition:ui::PAGE_TRANSITION_TYPED];
-}
-
-- (Tab*)addSelectedTabWithURL:(const GURL&)url
-                   transition:(ui::PageTransition)transition {
-  return [self addSelectedTabWithURL:url
                       atIndex:self.tabModel.count
-                          transition:transition];
+                   transition:ui::PAGE_TRANSITION_TYPED];
 }
 
 - (Tab*)addSelectedTabWithURL:(const GURL&)url
                       atIndex:(NSUInteger)position
                    transition:(ui::PageTransition)transition {
   return [self addSelectedTabWithURL:url
+                            postData:NULL
                              atIndex:position
-                          transition:transition
-                  tabAddedCompletion:nil];
+                          transition:transition];
 }
 
-- (Tab*)addSelectedTabWithURL:(const GURL&)url
-                      atIndex:(NSUInteger)position
-                   transition:(ui::PageTransition)transition
-           tabAddedCompletion:(ProceduralBlock)tabAddedCompletion {
-  return [self addSelectedTabWithURL:url
-                            postData:NULL
-                             atIndex:position
-                          transition:transition
-                  tabAddedCompletion:tabAddedCompletion];
+- (void)appendTabAddedCompletion:(ProceduralBlock)tabAddedCompletion {
+  if (tabAddedCompletion) {
+    if (self.foregroundTabWasAddedCompletionBlock) {
+      ProceduralBlock oldForegroundTabWasAddedCompletionBlock =
+          self.foregroundTabWasAddedCompletionBlock;
+      self.foregroundTabWasAddedCompletionBlock = ^{
+        oldForegroundTabWasAddedCompletionBlock();
+        tabAddedCompletion();
+      };
+    } else {
+      self.foregroundTabWasAddedCompletionBlock = tabAddedCompletion;
+    }
+  }
 }
 
 - (void)expectNewForegroundTab {
@@ -2852,15 +2849,13 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint {
   return [self addSelectedTabWithURL:url
                             postData:postData
                              atIndex:self.tabModel.count
-                          transition:transition
-                  tabAddedCompletion:nil];
+                          transition:transition];
 }
 
 - (Tab*)addSelectedTabWithURL:(const GURL&)URL
                      postData:(TemplateURLRef::PostContent*)postData
                       atIndex:(NSUInteger)position
-                   transition:(ui::PageTransition)transition
-           tabAddedCompletion:(ProceduralBlock)tabAddedCompletion {
+                   transition:(ui::PageTransition)transition {
   if (position == NSNotFound)
     position = self.tabModel.count;
   DCHECK(position <= self.tabModel.count);
@@ -2877,19 +2872,6 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint {
     params.extra_headers = @{@"Content-Type" : contentType};
   }
 
-  if (tabAddedCompletion) {
-    if (self.foregroundTabWasAddedCompletionBlock) {
-      ProceduralBlock oldForegroundTabWasAddedCompletionBlock =
-          self.foregroundTabWasAddedCompletionBlock;
-      self.foregroundTabWasAddedCompletionBlock = ^{
-        oldForegroundTabWasAddedCompletionBlock();
-        tabAddedCompletion();
-      };
-    } else {
-      self.foregroundTabWasAddedCompletionBlock = tabAddedCompletion;
-    }
-  }
-
   Tab* tab = [self.tabModel insertTabWithLoadParams:params
                                              opener:nil
                                         openedByDOM:NO
@@ -5323,7 +5305,9 @@ nativeContentHeaderHeightForPreloadController:(PreloadController*)controller
 - (void)captivePortalDetectorTabHelper:
             (CaptivePortalDetectorTabHelper*)tabHelper
                  connectWithLandingURL:(const GURL&)landingURL {
-  [self addSelectedTabWithURL:landingURL transition:ui::PAGE_TRANSITION_TYPED];
+  [self addSelectedTabWithURL:landingURL
+                      atIndex:self.tabModel.count
+                   transition:ui::PAGE_TRANSITION_TYPED];
 }
 
 #pragma mark - PageInfoPresentation