[iOS] Factor AppUrlLoadingService for Browser

Refactors the AppUrlLoadingService to get a Browser from its delegate
and not care about TabModel. There should be no functional changes.

Also :
 - Updates TestAppUrlLoadingService to use correct variable naming.

Bug: 1046373
Change-Id: I1da8a1033b0bf26972f07908bbe815c6b9d8f4c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2094308
Commit-Queue: Mark Cogan <marq@chromium.org>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748683}

ios/chrome/browser/ui/main/scene_controller.mm

@@ -900,7 +900,7 @@ const NSTimeInterval kDisplayPromoDelay = 0.1;
 }
 
 - (NSString*)currentPageSyncedUserName {
-  ChromeBrowserState* browserState = self.currentBrowserState;
+  ChromeBrowserState* browserState = self.currentInterface.browserState;
   if (browserState->IsOffTheRecord())
     return nil;
   signin::IdentityManager* identity_manager =
@@ -1353,12 +1353,8 @@ const NSTimeInterval kDisplayPromoDelay = 0.1;
                                           focusOmnibox:focusOmnibox];
 }
 
-- (ChromeBrowserState*)currentBrowserState {
-  return self.browserViewWrangler.currentInterface.browserState;
-}
-
-- (TabModel*)currentTabModel {
-  return self.browserViewWrangler.currentInterface.bvc.tabModel;
+- (Browser*)currentBrowserForURLLoading {
+  return self.currentInterface.browser;
 }
 
 // Asks the respective Snapshot helper to update the snapshot for the active
@@ -1504,7 +1500,7 @@ const NSTimeInterval kDisplayPromoDelay = 0.1;
     didDetachWebState:(web::WebState*)webState
               atIndex:(int)atIndex {
   // Do nothing on initialization.
-  if (![self currentTabModel].webStateList)
+  if (!self.currentInterface.browser)
     return;
 
   if (notifiedWebStateList->empty()) {
@@ -1540,11 +1536,13 @@ const NSTimeInterval kDisplayPromoDelay = 0.1;
   // Nothing to do here. The next user action (like clicking on an existing
   // regular tab or creating a new incognito tab from the settings menu) will
   // take care of the logic to mode switch.
-  if (self.tabSwitcherIsActive || ![self.currentTabModel isOffTheRecord]) {
+  if (self.tabSwitcherIsActive || !self.currentInterface.incognito) {
     return;
   }
 
-  if ([self.currentTabModel count] == 0U) {
+  WebStateList* currentWebStateList =
+      self.currentInterface.browser->GetWebStateList();
+  if (currentWebStateList->empty()) {
     [self showTabSwitcher];
   } else {
     [self setCurrentInterfaceForMode:ApplicationMode::NORMAL];
@@ -1559,7 +1557,7 @@ const NSTimeInterval kDisplayPromoDelay = 0.1;
   // closure of tabs from the main tab model when the main tab model is not
   // current.
   // Nothing to do here.
-  if (self.tabSwitcherIsActive || [self.currentTabModel isOffTheRecord]) {
+  if (self.tabSwitcherIsActive || self.currentInterface.incognito) {
     return;
   }
 
@@ -1587,7 +1585,7 @@ const NSTimeInterval kDisplayPromoDelay = 0.1;
   // until the callback is received.
   BrowsingDataRemover* browsingDataRemover =
       BrowsingDataRemoverFactory::GetForBrowserStateIfExists(
-          self.currentBrowserState);
+          self.currentInterface.browser->GetBrowserState());
   if (browsingDataRemover && browsingDataRemover->IsRemoving())
     return;
 

ios/chrome/browser/url_loading/app_url_loading_service.h

@@ -13,11 +13,9 @@
 #include "ios/chrome/app/application_mode.h"
 #include "ui/base/page_transition_types.h"
 
-class ChromeBrowserState;
+class Browser;
 struct UrlLoadParams;
 
-@class TabModel;
-
 // Objective-C delegate for AppUrlLoadingService.
 @protocol AppURLLoadingServiceDelegate
 
@@ -49,9 +47,8 @@ struct UrlLoadParams;
 // doing unnecessary work related to showing the previously selected tab.
 - (void)expectNewForegroundTabForMode:(ApplicationMode)targetMode;
 
-// TODO(crbug.com/907527): refactor to remove these and most methods above.
-- (ChromeBrowserState*)currentBrowserState;
-- (TabModel*)currentTabModel;
+// TODO(crbug.com/907527): refactor to remove this and most methods above.
+@property(nonatomic, readonly) Browser* currentBrowserForURLLoading;
 
 @end
 
@@ -66,8 +63,8 @@ class AppUrlLoadingService {
   // Opens a url based on |params| in a new tab.
   virtual void LoadUrlInNewTab(const UrlLoadParams& params);
 
-  // Returns the current browser state.
-  virtual ChromeBrowserState* GetCurrentBrowserState();
+  // Returns the current active browser in the scene owning this object.
+  virtual Browser* GetCurrentBrowser();
 
  private:
   __weak id<AppURLLoadingServiceDelegate> delegate_;

ios/chrome/browser/url_loading/app_url_loading_service.mm

@@ -27,6 +27,9 @@ void AppUrlLoadingService::SetDelegate(
 void AppUrlLoadingService::LoadUrlInNewTab(const UrlLoadParams& params) {
   DCHECK(delegate_);
 
+  ChromeBrowserState* current_browser_state =
+      delegate_.currentBrowserForURLLoading->GetBrowserState();
+
   if (params.web_params.url.is_valid()) {
     UrlLoadParams saved_params = params;
     saved_params.web_params.transition_type = ui::PAGE_TRANSITION_TYPED;
@@ -46,19 +49,18 @@ void AppUrlLoadingService::LoadUrlInNewTab(const UrlLoadParams& params) {
       [delegate_
           dismissModalDialogsWithCompletion:^{
             [delegate_ setCurrentInterfaceForMode:mode];
-            UrlLoadingServiceFactory::GetForBrowserState(
-                [delegate_ currentBrowserState])
+            UrlLoadingServiceFactory::GetForBrowserState(current_browser_state)
                 ->Load(saved_params);
           }
                              dismissOmnibox:YES];
     }
   } else {
-    if ([delegate_ currentBrowserState] -> IsOffTheRecord() !=
-                                               params.in_incognito) {
+    if (current_browser_state->IsOffTheRecord() != params.in_incognito) {
       // Must take a snapshot of the tab before we switch the incognito mode
       // because the currentTab will change after the switch.
       web::WebState* currentWebState =
-          [delegate_ currentTabModel].webStateList->GetActiveWebState();
+          delegate_.currentBrowserForURLLoading->GetWebStateList()
+              ->GetActiveWebState();
       if (currentWebState) {
         SnapshotTabHelper::FromWebState(currentWebState)
             ->UpdateSnapshotWithCallback(nil);
@@ -81,6 +83,6 @@ void AppUrlLoadingService::LoadUrlInNewTab(const UrlLoadParams& params) {
   }
 }
 
-ChromeBrowserState* AppUrlLoadingService::GetCurrentBrowserState() {
-  return [delegate_ currentBrowserState];
+Browser* AppUrlLoadingService::GetCurrentBrowser() {
+  return [delegate_ currentBrowserForURLLoading];
 }

ios/chrome/browser/url_loading/test_app_url_loading_service.h

@@ -8,7 +8,7 @@
 #include "ios/chrome/browser/url_loading/app_url_loading_service.h"
 #import "ios/chrome/browser/url_loading/url_loading_params.h"
 
-class ChromeBrowserState;
+class Browser;
 
 // Service used to manage url loading at application level.
 class TestAppUrlLoadingService : public AppUrlLoadingService {
@@ -20,14 +20,14 @@ class TestAppUrlLoadingService : public AppUrlLoadingService {
   void LoadUrlInNewTab(const UrlLoadParams& params) override;
 
   // Returns the current browser state.
-  ChromeBrowserState* GetCurrentBrowserState() override;
+  Browser* GetCurrentBrowser() override;
 
   // These are the last parameters passed to |LoadUrlInNewTab|.
-  UrlLoadParams last_params;
-  int load_new_tab_call_count = 0;
+  UrlLoadParams last_params_;
+  int load_new_tab_call_count_ = 0;
 
   // This can be set by the test.
-  ChromeBrowserState* currentBrowserState;
+  Browser* current_browser_;
 };
 
 #endif  // IOS_CHROME_BROWSER_URL_LOADING_APP_URL_LOADING_SERVICE_H_

ios/chrome/browser/url_loading/test_app_url_loading_service.mm

@@ -11,10 +11,10 @@
 TestAppUrlLoadingService::TestAppUrlLoadingService() {}
 
 void TestAppUrlLoadingService::LoadUrlInNewTab(const UrlLoadParams& params) {
-  last_params = params;
-  load_new_tab_call_count++;
+  last_params_ = params;
+  load_new_tab_call_count_++;
 }
 
-ChromeBrowserState* TestAppUrlLoadingService::GetCurrentBrowserState() {
-  return currentBrowserState;
+Browser* TestAppUrlLoadingService::GetCurrentBrowser() {
+  return current_browser_;
 }

ios/chrome/browser/url_loading/url_loading_service.mm

@@ -255,6 +255,8 @@ void UrlLoadingService::LoadUrlInNewTab(const UrlLoadParams& params) {
   DCHECK(delegate_);
   DCHECK(browser_);
   ChromeBrowserState* browser_state = browser_->GetBrowserState();
+  ChromeBrowserState* active_browser_state =
+      app_service_->GetCurrentBrowser()->GetBrowserState();
 
   // Two UrlLoadingServices exist, normal and incognito.  Handle two special
   // cases that need to be sent up to the AppUrlLoadingService:
@@ -264,8 +266,7 @@ void UrlLoadingService::LoadUrlInNewTab(const UrlLoadParams& params) {
   // so the AppUrlLoadingService needs to switch modes before loading the URL.
   if (params.in_incognito != browser_state->IsOffTheRecord() ||
       (!params.in_background() &&
-       params.in_incognito !=
-           app_service_->GetCurrentBrowserState()->IsOffTheRecord())) {
+       params.in_incognito != active_browser_state->IsOffTheRecord())) {
     // When sending a load request that switches modes, ensure the tab
     // ends up appended to the end of the model, not just next to what is
     // currently selected in the other mode. This is done with the |append_to|

ios/chrome/browser/url_loading/url_loading_service_unittest.mm

@@ -69,7 +69,7 @@ class URLLoadingServiceTest : public BlockCleanupTest {
         otr_service_(
             UrlLoadingServiceFactory::GetForBrowserState(otr_browser_state_)) {
     // Configure app service.
-    app_service_->currentBrowserState = chrome_browser_state_;
+    app_service_->current_browser_ = browser_.get();
 
     // Disable web usage on both browsers
     WebUsageEnablerBrowserAgent::CreateForBrowser(browser_.get());
@@ -153,7 +153,7 @@ TEST_F(URLLoadingServiceTest, TestSwitchToTab) {
   service_->Load(
       UrlLoadParams::SwitchToTab(web::NavigationManager::WebLoadParams(url)));
   EXPECT_EQ(web_state_ptr_2, web_state_list->GetActiveWebState());
-  EXPECT_EQ(0, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(0, app_service_->load_new_tab_call_count_);
 }
 
 // Tests that switch to open tab from the NTP close it if it doesn't have
@@ -185,7 +185,7 @@ TEST_F(URLLoadingServiceTest, TestSwitchToTabFromNTP) {
       UrlLoadParams::SwitchToTab(web::NavigationManager::WebLoadParams(url)));
   EXPECT_EQ(web_state_ptr_2, web_state_list->GetActiveWebState());
   EXPECT_EQ(1, web_state_list->count());
-  EXPECT_EQ(0, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(0, app_service_->load_new_tab_call_count_);
 }
 
 // Tests that trying to switch to a closed tab open from the NTP opens it in the
@@ -208,7 +208,7 @@ TEST_F(URLLoadingServiceTest, TestSwitchToClosedTab) {
       UrlLoadParams::SwitchToTab(web::NavigationManager::WebLoadParams(url)));
   EXPECT_EQ(1, web_state_list->count());
   EXPECT_EQ(web_state_ptr, web_state_list->GetActiveWebState());
-  EXPECT_EQ(0, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(0, app_service_->load_new_tab_call_count_);
 }
 
 // Tests open a new url in the NTP or the current tab.
@@ -237,7 +237,7 @@ TEST_F(URLLoadingServiceTest, TestOpenInCurrentTab) {
   EXPECT_EQ(1, web_state_list->count());
 
   // Check that we had no app level redirection.
-  EXPECT_EQ(0, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(0, app_service_->load_new_tab_call_count_);
 }
 
 // Tests opening a url in a new tab.
@@ -258,7 +258,7 @@ TEST_F(URLLoadingServiceTest, TestOpenInNewTab) {
   EXPECT_EQ(2, web_state_list->count());
 
   // Check that we had no app level redirection.
-  EXPECT_EQ(0, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(0, app_service_->load_new_tab_call_count_);
 }
 
 // Tests open a new url in the current incognito tab.
@@ -269,9 +269,9 @@ TEST_F(URLLoadingServiceTest, TestOpenInCurrentIncognitoTab) {
   ASSERT_EQ(0, otr_web_state_list->count());
 
   // Make app level to be otr.
-  ChromeBrowserState* otr_browser_state =
-      chrome_browser_state_->GetOffTheRecordChromeBrowserState();
-  app_service_->currentBrowserState = otr_browser_state;
+  std::unique_ptr<TestBrowser> otr_browser = std::make_unique<TestBrowser>(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  app_service_->current_browser_ = otr_browser.get();
 
   // Set a new tab.
   GURL newtab("chrome://newtab");
@@ -301,7 +301,7 @@ TEST_F(URLLoadingServiceTest, TestOpenInCurrentIncognitoTab) {
   EXPECT_EQ(1, otr_web_state_list->count());
 
   // Check that we had no app level redirection.
-  EXPECT_EQ(0, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(0, app_service_->load_new_tab_call_count_);
 }
 
 // Tests opening a url in a new incognito tab.
@@ -311,9 +311,9 @@ TEST_F(URLLoadingServiceTest, TestOpenInNewIncognitoTab) {
   WebStateList* otr_web_state_list = otr_browser_->GetWebStateList();
   ASSERT_EQ(0, otr_web_state_list->count());
 
-  ChromeBrowserState* otr_browser_state =
-      chrome_browser_state_->GetOffTheRecordChromeBrowserState();
-  app_service_->currentBrowserState = otr_browser_state;
+  std::unique_ptr<TestBrowser> otr_browser = std::make_unique<TestBrowser>(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  app_service_->current_browser_ = otr_browser.get();
 
   GURL url1("http://test/1");
   UrlLoadParams params1 =
@@ -332,7 +332,7 @@ TEST_F(URLLoadingServiceTest, TestOpenInNewIncognitoTab) {
   EXPECT_EQ(2, otr_web_state_list->count());
 
   // Check if we had any app level redirection.
-  EXPECT_EQ(0, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(0, app_service_->load_new_tab_call_count_);
 }
 
 // Test opening a normal url in new tab with incognito service.
@@ -342,9 +342,9 @@ TEST_F(URLLoadingServiceTest, TestOpenNormalInNewTabWithIncognitoService) {
   WebStateList* otr_web_state_list = otr_browser_->GetWebStateList();
   ASSERT_EQ(0, otr_web_state_list->count());
 
-  ChromeBrowserState* otr_browser_state =
-      chrome_browser_state_->GetOffTheRecordChromeBrowserState();
-  app_service_->currentBrowserState = otr_browser_state;
+  std::unique_ptr<TestBrowser> otr_browser = std::make_unique<TestBrowser>(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  app_service_->current_browser_ = otr_browser.get();
 
   // Send to right service.
   GURL url1("http://test/1");
@@ -365,7 +365,7 @@ TEST_F(URLLoadingServiceTest, TestOpenNormalInNewTabWithIncognitoService) {
   EXPECT_EQ(1, otr_web_state_list->count());
 
   // Check that had one app level redirection.
-  EXPECT_EQ(1, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(1, app_service_->load_new_tab_call_count_);
 }
 
 // Test opening an incognito url in new tab with normal service.
@@ -375,7 +375,7 @@ TEST_F(URLLoadingServiceTest, TestOpenIncognitoInNewTabWithNormalService) {
   WebStateList* otr_web_state_list = otr_browser_->GetWebStateList();
   ASSERT_EQ(0, otr_web_state_list->count());
 
-  app_service_->currentBrowserState = chrome_browser_state_;
+  app_service_->current_browser_ = browser_.get();
 
   // Send to wrong service.
   GURL url1("http://test/1");
@@ -396,7 +396,7 @@ TEST_F(URLLoadingServiceTest, TestOpenIncognitoInNewTabWithNormalService) {
   EXPECT_EQ(0, otr_web_state_list->count());
 
   // Check that we had one app level redirection.
-  EXPECT_EQ(1, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(1, app_service_->load_new_tab_call_count_);
 }
 
 // Test opening an incognito url in new tab with normal service using load
@@ -407,7 +407,7 @@ TEST_F(URLLoadingServiceTest, TestOpenIncognitoInNewTabWithLoadStrategy) {
   WebStateList* otr_web_state_list = otr_browser_->GetWebStateList();
   ASSERT_EQ(0, otr_web_state_list->count());
 
-  app_service_->currentBrowserState = chrome_browser_state_;
+  app_service_->current_browser_ = browser_.get();
 
   // Send to normal service.
   GURL url1("http://test/1");
@@ -419,7 +419,7 @@ TEST_F(URLLoadingServiceTest, TestOpenIncognitoInNewTabWithLoadStrategy) {
   EXPECT_EQ(0, otr_web_state_list->count());
 
   // Check that we had one app level redirection.
-  EXPECT_EQ(1, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(1, app_service_->load_new_tab_call_count_);
 }
 
 // Test opening an incognito url in current tab with normal service using load
@@ -431,9 +431,9 @@ TEST_F(URLLoadingServiceTest, TestOpenIncognitoInCurrentTabWithLoadStrategy) {
   ASSERT_EQ(0, otr_web_state_list->count());
 
   // Make app level to be otr.
-  ChromeBrowserState* otr_browser_state =
-      chrome_browser_state_->GetOffTheRecordChromeBrowserState();
-  app_service_->currentBrowserState = otr_browser_state;
+  std::unique_ptr<TestBrowser> otr_browser = std::make_unique<TestBrowser>(
+      chrome_browser_state_->GetOffTheRecordChromeBrowserState());
+  app_service_->current_browser_ = otr_browser.get();
 
   // Set a new incognito tab.
   GURL newtab("chrome://newtab");
@@ -463,7 +463,7 @@ TEST_F(URLLoadingServiceTest, TestOpenIncognitoInCurrentTabWithLoadStrategy) {
   EXPECT_EQ(1, otr_web_state_list->count());
 
   // Check that we had no app level redirection.
-  EXPECT_EQ(0, app_service_->load_new_tab_call_count);
+  EXPECT_EQ(0, app_service_->load_new_tab_call_count_);
 }
 
 }  // namespace