Check if browser is visible when launching apps.

WebState::IsVisible returns true if another VC is presented over it, but as it is not user visible, we should still prevent launching external applications. Bug: 1496222 Change-Id: I2fcd4c30c3ccdba4de1970f3ab3832e9b153cdc7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4987301 Reviewed-by: Gauthier Ambard <gambard@chromium.org> Auto-Submit: Olivier Robin <olivierrobin@chromium.org> Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com> Reviewed-by: Ali Juma <ajuma@chromium.org> Commit-Queue: Olivier Robin <olivierrobin@chromium.org> Cr-Commit-Position: refs/heads/main@{#1216892}
chromium · Oct 30, 2023 · c164bd4 · c164bd4
1 parent a34bdf7
commit c164bd4
Show file tree

Hide file tree

Showing 9 changed files with 123 additions and 5 deletions.
diff --git a/ios/chrome/browser/app_launcher/model/BUILD.gn b/ios/chrome/browser/app_launcher/model/BUILD.gn
@@ -12,6 +12,7 @@ source_set("model") {
     "app_launcher_browser_agent.mm",
     "app_launcher_tab_helper.h",
     "app_launcher_tab_helper.mm",
+    "app_launcher_tab_helper_browser_presentation_provider.h",
     "app_launcher_tab_helper_delegate.h",
     "app_launching_state.h",
     "app_launching_state.mm",

diff --git a/ios/chrome/browser/app_launcher/model/app_launcher_browser_agent_unittest.mm b/ios/chrome/browser/app_launcher/model/app_launcher_browser_agent_unittest.mm
@@ -9,6 +9,7 @@
 
 #import "base/test/metrics/histogram_tester.h"
 #import "ios/chrome/browser/app_launcher/model/app_launcher_tab_helper.h"
+#import "ios/chrome/browser/app_launcher/model/app_launcher_tab_helper_browser_presentation_provider.h"
 #import "ios/chrome/browser/app_launcher/model/fake_app_launcher_abuse_detector.h"
 #import "ios/chrome/browser/overlays/public/overlay_callback_manager.h"
 #import "ios/chrome/browser/overlays/public/overlay_request.h"
@@ -76,6 +77,14 @@
     abuse_detectors_[web_state] = abuse_detector;
     AppLauncherTabHelper::CreateForWebState(web_state, abuse_detector,
                                             incognito);
+    app_lancher_tab_helper_browser_presentation_provider_ = OCMProtocolMock(
+        @protocol(AppLauncherTabHelperBrowserPresentationProvider));
+    [[[app_lancher_tab_helper_browser_presentation_provider_ stub]
+        andReturnValue:@NO] isBrowserPresentingUI];
+    AppLauncherTabHelper::FromWebState(web_state)
+        ->SetBrowserPresentationProvider(
+            app_lancher_tab_helper_browser_presentation_provider_);
+
     // Insert the WebState into the Browser's WebStateList.
     int index = browser_->GetWebStateList()->count();
     browser_->GetWebStateList()->InsertWebState(
@@ -107,6 +116,7 @@ bool IsShowingDialog(
   std::unique_ptr<TestBrowser> browser_;
   std::map<web::WebState*, FakeAppLauncherAbuseDetector*> abuse_detectors_;
   id application_ = nil;
+  id app_lancher_tab_helper_browser_presentation_provider_ = nil;
 };
 
 // Tests that the browser agent shows an alert for app store URLs.

diff --git a/ios/chrome/browser/app_launcher/model/app_launcher_tab_helper.h b/ios/chrome/browser/app_launcher/model/app_launcher_tab_helper.h
@@ -8,10 +8,11 @@
 #import "ios/web/public/navigation/web_state_policy_decider.h"
 #import "ios/web/public/web_state_user_data.h"
 
-class AppLauncherTabHelperDelegate;
 @class AppLauncherAbuseDetector;
-class GURL;
 enum class AppLauncherAlertCause;
+@protocol AppLauncherTabHelperBrowserPresentationProvider;
+class AppLauncherTabHelperDelegate;
+class GURL;
 
 // A tab helper that handles requests to launch another application.
 class AppLauncherTabHelper
@@ -30,6 +31,11 @@ class AppLauncherTabHelper
   // Sets the delegate.
   void SetDelegate(AppLauncherTabHelperDelegate* delegate);
 
+  // Sets the provider to retrieve the browser presentation state.
+  void SetBrowserPresentationProvider(
+      id<AppLauncherTabHelperBrowserPresentationProvider>
+          browser_presentation_provider);
+
   // Requests to open the application with `url`.
   // The method checks if the application for `url` has been opened repeatedly
   // by the `source_page_url` page in a short time frame, in that case a prompt
@@ -112,6 +118,10 @@ class AppLauncherTabHelper
   // Used to launch apps and present UI.
   AppLauncherTabHelperDelegate* delegate_ = nullptr;
 
+  // Used to know if the browser is currently presenting another VC.
+  __weak id<AppLauncherTabHelperBrowserPresentationProvider>
+      browser_presentation_provider_ = nil;
+
   // Returns whether there is a prompt shown by `RequestToOpenUrl` or not.
   bool is_prompt_active_ = false;
 

diff --git a/ios/chrome/browser/app_launcher/model/app_launcher_tab_helper.mm b/ios/chrome/browser/app_launcher/model/app_launcher_tab_helper.mm
@@ -12,6 +12,7 @@
 #import "components/policy/core/browser/url_blocklist_manager.h"
 #import "components/reading_list/core/reading_list_model.h"
 #import "ios/chrome/browser/app_launcher/model/app_launcher_abuse_detector.h"
+#import "ios/chrome/browser/app_launcher/model/app_launcher_tab_helper_browser_presentation_provider.h"
 #import "ios/chrome/browser/app_launcher/model/app_launcher_tab_helper_delegate.h"
 #import "ios/chrome/browser/policy_url_blocking/model/policy_url_blocking_service.h"
 #import "ios/chrome/browser/policy_url_blocking/model/policy_url_blocking_util.h"
@@ -90,6 +91,12 @@ bool HasChromeAppScheme(const GURL& app_url) {
   delegate_ = delegate;
 }
 
+void AppLauncherTabHelper::SetBrowserPresentationProvider(
+    id<AppLauncherTabHelperBrowserPresentationProvider>
+        browser_presentation_provider) {
+  browser_presentation_provider_ = browser_presentation_provider;
+}
+
 void AppLauncherTabHelper::RequestToLaunchApp(const GURL& url,
                                               const GURL& source_page_url,
                                               bool link_transition,
@@ -98,7 +105,8 @@ bool HasChromeAppScheme(const GURL& app_url) {
   // web_state is not visible.
   if ([[UIApplication sharedApplication] applicationState] !=
           UIApplicationStateActive ||
-      !web_state_->IsVisible()) {
+      !web_state_->IsVisible() || !browser_presentation_provider_ ||
+      [browser_presentation_provider_ isBrowserPresentingUI]) {
     return;
   }
 

diff --git a/...chrome/browser/app_launcher/model/app_launcher_tab_helper_browser_presentation_provider.h b/...chrome/browser/app_launcher/model/app_launcher_tab_helper_browser_presentation_provider.h
@@ -0,0 +1,17 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef IOS_CHROME_BROWSER_APP_LAUNCHER_MODEL_APP_LAUNCHER_TAB_HELPER_BROWSER_PRESENTATION_PROVIDER_H_
+#define IOS_CHROME_BROWSER_APP_LAUNCHER_MODEL_APP_LAUNCHER_TAB_HELPER_BROWSER_PRESENTATION_PROVIDER_H_
+
+// Protocol to retrieve whether the browser is presenting another VC.
+@protocol AppLauncherTabHelperBrowserPresentationProvider
+
+// Whether the browser is currently presenting a UI (and so the webState is not
+// on top). This UI may or may not occlude the web view.
+- (BOOL)isBrowserPresentingUI;
+
+@end
+
+#endif  // IOS_CHROME_BROWSER_APP_LAUNCHER_MODEL_APP_LAUNCHER_TAB_HELPER_BROWSER_PRESENTATION_PROVIDER_H_
diff --git a/ios/chrome/browser/app_launcher/model/app_launcher_tab_helper_unittest.mm b/ios/chrome/browser/app_launcher/model/app_launcher_tab_helper_unittest.mm
@@ -16,6 +16,7 @@
 #import "components/policy/policy_constants.h"
 #import "components/reading_list/core/reading_list_entry.h"
 #import "components/reading_list/core/reading_list_model.h"
+#import "ios/chrome/browser/app_launcher/model/app_launcher_tab_helper_browser_presentation_provider.h"
 #import "ios/chrome/browser/app_launcher/model/app_launcher_tab_helper_delegate.h"
 #import "ios/chrome/browser/app_launcher/model/fake_app_launcher_abuse_detector.h"
 #import "ios/chrome/browser/policy/enterprise_policy_test_helper.h"
@@ -121,6 +122,23 @@ void DiscardNonCommittedItems() override {}
 
 }  // namespace
 
+// A fake AppLauncherTabHelperBrowserPresentationProvider to be used by the
+// AppLaunchTabHelper with customizable presentingUI property.
+@interface FakeAppLauncherTabHelperBrowserPresentationProvider
+    : NSObject <AppLauncherTabHelperBrowserPresentationProvider>
+
+@property(nonatomic, assign) BOOL presentingUI;
+
+@end
+
+@implementation FakeAppLauncherTabHelperBrowserPresentationProvider
+
+- (BOOL)isBrowserPresentingUI {
+  return _presentingUI;
+}
+
+@end
+
 // Test fixture for AppLauncherTabHelper class.
 class AppLauncherTabHelperTest : public PlatformTest {
  protected:
@@ -143,8 +161,11 @@ void SetUp() override {
     web_state_.SetCurrentURL(GURL("https://chromium.org"));
     web_state_.SetBrowserState(browser_state_.get());
     web_state_.WasShown();
+    browser_presentation_provider_ =
+        [[FakeAppLauncherTabHelperBrowserPresentationProvider alloc] init];
     tab_helper_ = AppLauncherTabHelper::FromWebState(&web_state_);
     tab_helper_->SetDelegate(&delegate_);
+    tab_helper_->SetBrowserPresentationProvider(browser_presentation_provider_);
   }
 
   [[nodiscard]] bool TestShouldAllowRequest(
@@ -228,6 +249,8 @@ bool TestReadingListUpdate(bool is_app_blocked,
   FakeNavigationManager* navigation_manager_ = nullptr;
   FakeAppLauncherAbuseDetector* abuse_detector_ = nil;
   FakeAppLauncherTabHelperDelegate delegate_;
+  FakeAppLauncherTabHelperBrowserPresentationProvider*
+      browser_presentation_provider_;
   AppLauncherTabHelper* tab_helper_ = nullptr;
 };
 
@@ -478,6 +501,39 @@ bool TestReadingListUpdate(bool is_app_blocked,
   EXPECT_EQ(0U, delegate_.GetAppLaunchCount());
 }
 
+// Tests that if web_state is not shown or if there is a UI on top of it, no
+// request is triggered.
+TEST_F(AppLauncherTabHelperTest, WebStateNotShown) {
+  // Base case.
+  NSString* url_string = @"valid://1234";
+  EXPECT_FALSE(TestShouldAllowRequest(url_string, /*target_frame_is_main=*/true,
+                                      /*target_frame_is_cross_origin=*/false,
+                                      /*is_user_initiated=*/true));
+  EXPECT_EQ(1U, delegate_.GetAppLaunchCount());
+
+  // WebState hidden.
+  web_state_.WasHidden();
+  EXPECT_FALSE(TestShouldAllowRequest(url_string, /*target_frame_is_main=*/true,
+                                      /*target_frame_is_cross_origin=*/false,
+                                      /*is_user_initiated=*/true));
+  EXPECT_EQ(1U, delegate_.GetAppLaunchCount());
+
+  // Browser not visible.
+  web_state_.WasShown();
+  browser_presentation_provider_.presentingUI = YES;
+  EXPECT_FALSE(TestShouldAllowRequest(url_string, /*target_frame_is_main=*/true,
+                                      /*target_frame_is_cross_origin=*/false,
+                                      /*is_user_initiated=*/true));
+  EXPECT_EQ(1U, delegate_.GetAppLaunchCount());
+
+  // Base case to check.
+  browser_presentation_provider_.presentingUI = NO;
+  EXPECT_FALSE(TestShouldAllowRequest(url_string, /*target_frame_is_main=*/true,
+                                      /*target_frame_is_cross_origin=*/false,
+                                      /*is_user_initiated=*/true));
+  EXPECT_EQ(2U, delegate_.GetAppLaunchCount());
+}
+
 // Tests that when the last committed URL is invalid, the URL is only opened
 // when the last committed item is nil.
 // TODO(crbug.com/1172516): The test fails on device.

diff --git a/ios/chrome/browser/ui/browser_view/browser_coordinator.mm b/ios/chrome/browser/ui/browser_view/browser_coordinator.mm
@@ -24,8 +24,7 @@
 #import "components/segmentation_platform/embedder/default_model/device_switcher_result_dispatcher.h"
 #import "components/signin/ios/browser/active_state_manager.h"
 #import "components/translate/core/browser/translate_manager.h"
-#import "ios/chrome/browser/app_launcher/model/app_launcher_abuse_detector.h"
-#import "ios/chrome/browser/app_launcher/model/app_launcher_tab_helper.h"
+#import "ios/chrome/browser/app_launcher/model/app_launcher_tab_helper_browser_presentation_provider.h"
 #import "ios/chrome/browser/commerce/model/push_notification/push_notification_feature.h"
 #import "ios/chrome/browser/commerce/model/shopping_service_factory.h"
 #import "ios/chrome/browser/content_settings/model/host_content_settings_map_factory.h"
@@ -253,6 +252,7 @@
 }  // anonymous namespace
 
 @interface BrowserCoordinator () <
+    AppLauncherTabHelperBrowserPresentationProvider,
     AutofillAddCreditCardCoordinatorDelegate,
     BrowserCoordinatorCommands,
     BubblePresenterDelegate,
@@ -1457,6 +1457,7 @@ - (void)startTabLifeCycleMediator {
       TabInsertionBrowserAgent::FromBrowser(browser);
   tabLifecycleMediator.snapshotGeneratorDelegate = self;
   tabLifecycleMediator.overscrollActionsDelegate = self;
+  tabLifecycleMediator.appLauncherBrowserPresentationProvider = self;
 
   self.tabLifecycleMediator = tabLifecycleMediator;
 }
@@ -3204,4 +3205,10 @@ - (void)autofillAddCreditCardCoordinatorWantsToBeStopped:
   [self stopAutofillAddCreditCardCoordinator];
 }
 
+#pragma mark - AppLauncherTabHelperBrowserPresentationProvider
+
+- (BOOL)isBrowserPresentingUI {
+  return self.viewController.presentedViewController != nil;
+}
+
 @end
diff --git a/ios/chrome/browser/ui/browser_view/tab_lifecycle_mediator.h b/ios/chrome/browser/ui/browser_view/tab_lifecycle_mediator.h
@@ -7,6 +7,7 @@
 
 #import <UIKit/UIKit.h>
 
+@protocol AppLauncherTabHelperBrowserPresentationProvider;
 @class CommandDispatcher;
 @protocol DownloadManagerTabHelperDelegate;
 @class NewTabPageCoordinator;
@@ -52,6 +53,8 @@ class WebStateList;
     passwordControllerDelegate;
 @property(nonatomic, weak) id<SnapshotGeneratorDelegate>
     snapshotGeneratorDelegate;
+@property(nonatomic, weak) id<AppLauncherTabHelperBrowserPresentationProvider>
+    appLauncherBrowserPresentationProvider;
 
 // Creates an instance of the mediator. Delegates will be installed into all
 // existing web states in `webStateList`. While the mediator is alive,

diff --git a/ios/chrome/browser/ui/browser_view/tab_lifecycle_mediator.mm b/ios/chrome/browser/ui/browser_view/tab_lifecycle_mediator.mm
@@ -4,6 +4,7 @@
 
 #import "ios/chrome/browser/ui/browser_view/tab_lifecycle_mediator.h"
 
+#import "ios/chrome/browser/app_launcher/model/app_launcher_tab_helper.h"
 #import "ios/chrome/browser/autofill/autofill_tab_helper.h"
 #import "ios/chrome/browser/autofill/bottom_sheet/autofill_bottom_sheet_tab_helper.h"
 #import "ios/chrome/browser/commerce/model/price_notifications/price_notifications_iph_presenter.h"
@@ -166,6 +167,8 @@ - (void)installDependencyForWebState:(web::WebState*)webState {
     priceNotificationsTabHelper->SetPriceNotificationsIPHPresenter(
         _priceNotificationsIPHPresenter);
   }
+  AppLauncherTabHelper::FromWebState(webState)->SetBrowserPresentationProvider(
+      _appLauncherBrowserPresentationProvider);
 }
 
 - (void)uninstallDependencyForWebState:(web::WebState*)webState {
@@ -225,6 +228,9 @@ - (void)uninstallDependencyForWebState:(web::WebState*)webState {
   if (priceNotificationsTabHelper) {
     priceNotificationsTabHelper->SetPriceNotificationsIPHPresenter(nil);
   }
+
+  AppLauncherTabHelper::FromWebState(webState)->SetBrowserPresentationProvider(
+      nil);
 }
 
 @end