[iOS][PageInfo] Move showSecurityHelpPage to presentationCommands

This CL moves the `showSecurityHelpPage` method from the more generic PageInfoCommands implemented by BrowserCoordinator to PageInfoPresentationCommands defined by PageInfoCoordinator. The presentationCommands takes care of actions within the PageInfo UI and showing the security help page will only be triggered within PageInfo. Fixed: b:325878304 Bug: 1512580 Change-Id: Icc9dc6bd69c53843b685be2534f0440ec351a75f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5306314 Reviewed-by: Ewann Pellé <[email protected]> Commit-Queue: Filipa Senra <[email protected]> Reviewed-by: Gauthier Ambard <[email protected]> Cr-Commit-Position: refs/heads/main@{#1262621}
anhtai9710 · Feb 20, 2024 · 88b0eca · 88b0eca
1 parent 9122358
commit 88b0eca
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 13 deletions.
diff --git a/ios/chrome/browser/shared/public/commands/page_info_commands.h b/ios/chrome/browser/shared/public/commands/page_info_commands.h
@@ -17,9 +17,6 @@
 // Hides the page security info.
 - (void)hidePageInfo;
 
-// Shows the security help page.
-- (void)showSecurityHelpPage;
-
 @end
 
 #endif  // IOS_CHROME_BROWSER_SHARED_PUBLIC_COMMANDS_PAGE_INFO_COMMANDS_H_
diff --git a/ios/chrome/browser/ui/browser_view/browser_coordinator.mm b/ios/chrome/browser/ui/browser_view/browser_coordinator.mm
@@ -2196,13 +2196,6 @@ - (void)hidePageInfo {
   self.pageInfoCoordinator = nil;
 }
 
-- (void)showSecurityHelpPage {
-  UrlLoadParams params = UrlLoadParams::InNewTab(GURL(kPageInfoHelpCenterURL));
-  params.in_incognito = self.browser->GetBrowserState()->IsOffTheRecord();
-  _urlLoadingBrowserAgent->Load(params);
-  [self hidePageInfo];
-}
-
 #pragma mark - FormInputAccessoryCoordinatorNavigator
 
 - (void)openPasswordManager {

diff --git a/ios/chrome/browser/ui/page_info/page_info_coordinator.mm b/ios/chrome/browser/ui/page_info/page_info_coordinator.mm
@@ -9,6 +9,7 @@
 #import "ios/chrome/browser/page_info/about_this_site_service_factory.h"
 #import "ios/chrome/browser/shared/model/browser/browser.h"
 #import "ios/chrome/browser/shared/model/browser_state/chrome_browser_state.h"
+#import "ios/chrome/browser/shared/model/url/chrome_url_constants.h"
 #import "ios/chrome/browser/shared/model/web_state_list/web_state_list.h"
 #import "ios/chrome/browser/shared/public/commands/command_dispatcher.h"
 #import "ios/chrome/browser/shared/public/commands/page_info_commands.h"
@@ -102,9 +103,19 @@ - (void)showSecurityPage {
   _securityCoordinator = [[PageInfoSecurityCoordinator alloc]
       initWithBaseNavigationController:self.navigationController
                                browser:self.browser];
+  _securityCoordinator.pageInfoPresentationHandler = self;
   [_securityCoordinator start];
 }
 
+- (void)showSecurityHelpPage {
+  UrlLoadParams params = UrlLoadParams::InNewTab(GURL(kPageInfoHelpCenterURL));
+  params.in_incognito = self.browser->GetBrowserState()->IsOffTheRecord();
+  UrlLoadingBrowserAgent::FromBrowser(self.browser)->Load(params);
+  id<PageInfoCommands> pageInfoCommandsHandler =
+      HandlerForProtocol(self.dispatcher, PageInfoCommands);
+  [pageInfoCommandsHandler hidePageInfo];
+}
+
 - (void)showAboutThisSitePage:(GURL)URL {
   UrlLoadParams params = UrlLoadParams::InNewTab(URL);
   params.in_incognito = self.browser->GetBrowserState()->IsOffTheRecord();

diff --git a/ios/chrome/browser/ui/page_info/page_info_egtest.mm b/ios/chrome/browser/ui/page_info/page_info_egtest.mm
@@ -44,6 +44,19 @@
       kPageInfoMicrophoneSwitchAccessibilityIdentifier, isOn);
 }
 
+// Matcher for Security help center link in footer.
+id<GREYMatcher> SecurityHelpCenterLink() {
+  return grey_allOf(
+      // The link is within the security footer with ID
+      // `kPageInfoSecurityFooterAccessibilityIdentifier`.
+      grey_ancestor(
+          grey_accessibilityID(kPageInfoSecurityFooterAccessibilityIdentifier)),
+      // UIKit instantiates a `UIAccessibilityLinkSubelement` for the link
+      // element in the label with attributed string.
+      grey_kindOfClassName(@"UIAccessibilityLinkSubelement"),
+      grey_accessibilityTrait(UIAccessibilityTraitLink), nil);
+}
+
 void AddAboutThisSiteHint(GURL url) {
   [PageInfoAppInterface
       addAboutThisSiteHintForURL:
@@ -344,6 +357,15 @@ - (void)testLegacySecuritySection {
       selectElementWithMatcher:
           grey_accessibilityID(kPageInfoSecurityFooterAccessibilityIdentifier)]
       assertWithMatcher:grey_sufficientlyVisible()];
+
+  // Tap on the Learn more link.
+  [[EarlGrey selectElementWithMatcher:SecurityHelpCenterLink()]
+      performAction:grey_tap()];
+
+  // Check that the help center article was opened.
+  GREYAssertEqual(std::string("support.google.com"),
+                  [ChromeEarlGrey webStateVisibleURL].host(),
+                  @"Did not navigate to the help center article.");
 }
 
 // Tests the security section by checking that the correct connection label is

diff --git a/ios/chrome/browser/ui/page_info/page_info_presentation_commands.h b/ios/chrome/browser/ui/page_info/page_info_presentation_commands.h
@@ -13,6 +13,9 @@
 // Method invoked when the user requests more details about a page's security.
 - (void)showSecurityPage;
 
+// Method invoked when the user requests to see the security help page.
+- (void)showSecurityHelpPage;
+
 // Method invoked when the user requests more details about a page, i.e.
 // taps on AboutThisSite.
 - (void)showAboutThisSitePage:(GURL)URL;

diff --git a/ios/chrome/browser/ui/page_info/page_info_security_coordinator.h b/ios/chrome/browser/ui/page_info/page_info_security_coordinator.h
@@ -9,9 +9,15 @@
 
 #import "ios/chrome/browser/shared/coordinator/chrome_coordinator/chrome_coordinator.h"
 
+@protocol PageInfoPresentationCommands;
+
 // The coordinator for page info's security subpage.
 @interface PageInfoSecurityCoordinator : ChromeCoordinator
 
+// Handler for actions within the Page Info UI.
+@property(nonatomic, weak) id<PageInfoPresentationCommands>
+    pageInfoPresentationHandler;
+
 - (instancetype)initWithBaseNavigationController:
                     (UINavigationController*)navigationController
                                          browser:(Browser*)browser

diff --git a/ios/chrome/browser/ui/page_info/page_info_security_coordinator.mm b/ios/chrome/browser/ui/page_info/page_info_security_coordinator.mm
@@ -45,6 +45,8 @@ - (void)start {
 
   _viewController.pageInfoCommandsHandler = HandlerForProtocol(
       self.browser->GetCommandDispatcher(), PageInfoCommands);
+  _viewController.pageInfoPresentationHandler =
+      self.pageInfoPresentationHandler;
 
   [self.baseNavigationController pushViewController:_viewController
                                            animated:YES];

diff --git a/ios/chrome/browser/ui/page_info/page_info_security_view_controller.h b/ios/chrome/browser/ui/page_info/page_info_security_view_controller.h
@@ -11,6 +11,7 @@
 #import "ios/chrome/browser/ui/page_info/page_info_site_security_description.h"
 
 @protocol PageInfoCommands;
+@protocol PageInfoPresentationCommands;
 
 // View Controller for displaying the security subpage of page info.
 @interface PageInfoSecurityViewController
@@ -20,6 +21,10 @@
 // dismissing the entire UI.
 @property(nonatomic, weak) id<PageInfoCommands> pageInfoCommandsHandler;
 
+// Handler for actions within the Page Info UI.
+@property(nonatomic, weak) id<PageInfoPresentationCommands>
+    pageInfoPresentationHandler;
+
 // Designated initializer.
 - (instancetype)initWithSiteSecurityDescription:
     (PageInfoSiteSecurityDescription*)siteSecurityDescription

diff --git a/ios/chrome/browser/ui/page_info/page_info_security_view_controller.mm b/ios/chrome/browser/ui/page_info/page_info_security_view_controller.mm
@@ -15,6 +15,7 @@
 #import "ios/chrome/browser/shared/ui/table_view/table_view_utils.h"
 #import "ios/chrome/browser/ui/keyboard/UIKeyCommand+Chrome.h"
 #import "ios/chrome/browser/ui/page_info/page_info_constants.h"
+#import "ios/chrome/browser/ui/page_info/page_info_presentation_commands.h"
 #import "ios/chrome/common/ui/colors/semantic_color_names.h"
 #import "ios/chrome/common/ui/table_view/table_view_cells_constants.h"
 #import "ios/chrome/grit/ios_strings.h"
@@ -121,12 +122,12 @@ - (NSIndexPath*)tableView:(UITableView*)tableView
 
 - (void)tableView:(UITableView*)tableView
     didSelectRowAtIndexPath:(NSIndexPath*)indexPath {
-  CHECK(self.pageInfoCommandsHandler);
+  CHECK(self.pageInfoPresentationHandler);
   ItemIdentifier itemType = static_cast<ItemIdentifier>(
       [_dataSource itemIdentifierForIndexPath:indexPath].integerValue);
   switch (itemType) {
     case ItemIdentifierLearnMoreRow:
-      [self.pageInfoCommandsHandler showSecurityHelpPage];
+      [self.pageInfoPresentationHandler showSecurityHelpPage];
       break;
     default:
       break;

diff --git a/ios/chrome/browser/ui/page_info/page_info_view_controller.mm b/ios/chrome/browser/ui/page_info/page_info_view_controller.mm
@@ -261,7 +261,7 @@ - (UIView*)tableView:(UITableView*)tableView
 
 - (void)view:(TableViewLinkHeaderFooterView*)view didTapLinkURL:(CrURL*)URL {
   DCHECK(URL.gurl == GURL(kPageInfoHelpCenterURL));
-  [self.pageInfoCommandsHandler showSecurityHelpPage];
+  [self.pageInfoPresentationHandler showSecurityHelpPage];
 }
 
 #pragma mark - UIAdaptivePresentationControllerDelegate