[iOS] Stop Voiceover reading items on the WebView when omnibox is open

Currently, when the omnibox popup is open, Voiceover will highlight
and read the items from the WebView, even though those views are
covered by the popup. This is because the popup is just added to the
same parent VC (not presented). We need something to manually say that
the webview is covered.

Bug: 786940, 912419
Change-Id: I70ce9abcfff8c3f0e48aa990d383eabe78b6ef47
Reviewed-on: https://chromium-review.googlesource.com/c/1485951
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636754}

ios/chrome/browser/ui/BUILD.gn

@@ -338,6 +338,7 @@ source_set("ui_internal") {
     "//ios/chrome/browser/ui/ntp:ntp_controller",
     "//ios/chrome/browser/ui/ntp:util",
     "//ios/chrome/browser/ui/omnibox:omnibox_internal",
+    "//ios/chrome/browser/ui/omnibox/popup",
     "//ios/chrome/browser/ui/overscroll_actions",
     "//ios/chrome/browser/ui/page_info/requirements",
     "//ios/chrome/browser/ui/payments",

ios/chrome/browser/ui/browser_view_controller.mm

@@ -117,6 +117,7 @@
 #import "ios/chrome/browser/ui/ntp/new_tab_page_coordinator.h"
 #import "ios/chrome/browser/ui/ntp/new_tab_page_owning.h"
 #import "ios/chrome/browser/ui/ntp/ntp_util.h"
+#import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_presenter.h"
 #import "ios/chrome/browser/ui/overscroll_actions/overscroll_actions_controller.h"
 #import "ios/chrome/browser/ui/page_not_available_controller.h"
 #import "ios/chrome/browser/ui/payments/payment_request_manager.h"
@@ -372,6 +373,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
                                      MFMailComposeViewControllerDelegate,
                                      NetExportTabHelperDelegate,
                                      NewTabPageTabHelperDelegate,
+                                     OmniboxPopupPresenterDelegate,
                                      OverscrollActionsControllerDelegate,
                                      PasswordControllerDelegate,
                                      PreloadControllerDelegate,
@@ -1951,6 +1953,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
       [[PrimaryToolbarCoordinator alloc] initWithBrowserState:_browserState];
   self.primaryToolbarCoordinator = topToolbarCoordinator;
   topToolbarCoordinator.delegate = self;
+  topToolbarCoordinator.popupPresenterDelegate = self;
   topToolbarCoordinator.webStateList = self.tabModel.webStateList;
   topToolbarCoordinator.dispatcher = self.dispatcher;
   topToolbarCoordinator.commandDispatcher = self.commandDispatcher;
@@ -3520,6 +3523,31 @@ NSString* const kBrowserViewControllerSnackbarCategory =
   }
 }
 
+#pragma mark - OmniboxPopupPresenterDelegate methods.
+
+- (UIView*)popupParentViewForPresenter:(OmniboxPopupPresenter*)presenter {
+  return self.view;
+}
+
+- (UIViewController*)popupParentViewControllerForPresenter:
+    (OmniboxPopupPresenter*)presenter {
+  return self;
+}
+
+- (void)popupDidOpenForPresenter:(OmniboxPopupPresenter*)presenter {
+  self.contentArea.accessibilityElementsHidden = YES;
+  self.secondaryToolbarContainerView.accessibilityElementsHidden = YES;
+  self.infobarContainerCoordinator.legacyContainerView
+      .accessibilityElementsHidden = YES;
+}
+
+- (void)popupDidCloseForPresenter:(OmniboxPopupPresenter*)presenter {
+  self.contentArea.accessibilityElementsHidden = NO;
+  self.secondaryToolbarContainerView.accessibilityElementsHidden = NO;
+  self.infobarContainerCoordinator.legacyContainerView
+      .accessibilityElementsHidden = NO;
+}
+
 #pragma mark - OverscrollActionsControllerDelegate methods.
 
 - (void)overscrollActionsController:(OverscrollActionsController*)controller

ios/chrome/browser/ui/location_bar/location_bar_coordinator.h

@@ -20,7 +20,7 @@ class WebStateList;
 @protocol BrowserCommands;
 @protocol EditViewAnimatee;
 @protocol LocationBarAnimatee;
-@protocol OmniboxPopupPositioner;
+@protocol OmniboxPopupPresenterDelegate;
 @protocol ToolbarCoordinatorDelegate;
 
 // Location bar coordinator.
@@ -42,7 +42,8 @@ class WebStateList;
 // The web state list this ToolbarCoordinator is handling.
 @property(nonatomic, assign) WebStateList* webStateList;
 
-@property(nonatomic, weak) id<OmniboxPopupPositioner> popupPositioner;
+@property(nonatomic, weak) id<OmniboxPopupPresenterDelegate>
+    popupPresenterDelegate;
 
 // Start this coordinator.
 - (void)start;

ios/chrome/browser/ui/location_bar/location_bar_coordinator.mm

@@ -98,7 +98,6 @@ const int kLocationAuthorizationStatusCount = 4;
 @synthesize delegate = _delegate;
 @synthesize webStateList = _webStateList;
 @synthesize omniboxPopupCoordinator = _omniboxPopupCoordinator;
-@synthesize popupPositioner = _popupPositioner;
 @synthesize omniboxCoordinator = _omniboxCoordinator;
 
 #pragma mark - public
@@ -150,8 +149,8 @@ const int kLocationAuthorizationStatusCount = 4;
       didMoveToParentViewController:self.viewController];
   self.viewController.offsetProvider = [self.omniboxCoordinator offsetProvider];
 
-  self.omniboxPopupCoordinator =
-      [self.omniboxCoordinator createPopupCoordinator:self.popupPositioner];
+  self.omniboxPopupCoordinator = [self.omniboxCoordinator
+      createPopupCoordinator:self.popupPresenterDelegate];
   self.omniboxPopupCoordinator.dispatcher = self.dispatcher;
   self.omniboxPopupCoordinator.webStateList = self.webStateList;
   [self.omniboxPopupCoordinator start];

ios/chrome/browser/ui/location_bar/location_bar_generic_coordinator.h

@@ -20,7 +20,7 @@ class WebStateList;
 @protocol BrowserCommands;
 @protocol EditViewAnimatee;
 @protocol LocationBarAnimatee;
-@protocol OmniboxPopupPositioner;
+@protocol OmniboxPopupPresenterDelegate;
 @protocol ToolbarCoordinatorDelegate;
 
 @protocol LocationBarGenericCoordinator<NSObject,
@@ -41,7 +41,8 @@ class WebStateList;
 // The web state list this ToolbarCoordinator is handling.
 @property(nonatomic, assign) WebStateList* webStateList;
 
-@property(nonatomic, weak) id<OmniboxPopupPositioner> popupPositioner;
+@property(nonatomic, weak) id<OmniboxPopupPresenterDelegate>
+    popupPresenterDelegate;
 
 // Start this coordinator.
 - (void)start;

ios/chrome/browser/ui/omnibox/omnibox_coordinator.h

@@ -16,7 +16,7 @@ class WebOmniboxEditController;
 @class OmniboxPopupCoordinator;
 @class OmniboxTextFieldIOS;
 @protocol LocationBarOffsetProvider;
-@protocol OmniboxPopupPositioner;
+@protocol OmniboxPopupPresenterDelegate;
 
 // The coordinator for the omnibox.
 @interface OmniboxCoordinator : NSObject
@@ -59,7 +59,7 @@ class WebOmniboxEditController;
 // Creates a child popup coordinator. The popup coordinator is linked to the
 // |textField| through components code.
 - (OmniboxPopupCoordinator*)createPopupCoordinator:
-    (id<OmniboxPopupPositioner>)positioner;
+    (id<OmniboxPopupPresenterDelegate>)presenterDelegate;
 
 @end
 

ios/chrome/browser/ui/omnibox/omnibox_coordinator.mm

@@ -136,7 +136,7 @@
 }
 
 - (OmniboxPopupCoordinator*)createPopupCoordinator:
-    (id<OmniboxPopupPositioner>)positioner {
+    (id<OmniboxPopupPresenterDelegate>)presenterDelegate {
   std::unique_ptr<OmniboxPopupViewIOS> popupView =
       std::make_unique<OmniboxPopupViewIOS>(_editView->model(),
                                             _editView.get());
@@ -147,7 +147,7 @@
   OmniboxPopupCoordinator* coordinator =
       [[OmniboxPopupCoordinator alloc] initWithPopupView:std::move(popupView)];
   coordinator.browserState = self.browserState;
-  coordinator.positioner = positioner;
+  coordinator.presenterDelegate = presenterDelegate;
 
   return coordinator;
 }

ios/chrome/browser/ui/omnibox/popup/BUILD.gn

@@ -8,7 +8,6 @@ source_set("popup") {
     "omnibox_popup_coordinator.mm",
     "omnibox_popup_mediator.h",
     "omnibox_popup_mediator.mm",
-    "omnibox_popup_positioner.h",
     "omnibox_popup_presenter.h",
     "omnibox_popup_presenter.mm",
     "omnibox_popup_provider.h",

ios/chrome/browser/ui/omnibox/popup/omnibox_popup_coordinator.h

@@ -10,7 +10,7 @@
 #include <memory>
 
 @class CommandDispatcher;
-@protocol OmniboxPopupPositioner;
+@protocol OmniboxPopupPresenterDelegate;
 @protocol OmniboxFocuser;
 class OmniboxPopupViewIOS;
 
@@ -29,7 +29,7 @@ class WebStateList;
 // BrowserState.
 @property(nonatomic, assign) ios::ChromeBrowserState* browserState;
 // Positioner for the popup.
-@property(nonatomic, weak) id<OmniboxPopupPositioner> positioner;
+@property(nonatomic, weak) id<OmniboxPopupPresenterDelegate> presenterDelegate;
 // Whether this coordinator has results to show.
 @property(nonatomic, assign, readonly) BOOL hasResults;
 // Whether the popup is open.

ios/chrome/browser/ui/omnibox/popup/omnibox_popup_coordinator.mm

@@ -39,7 +39,6 @@
 @synthesize browserState = _browserState;
 @synthesize mediator = _mediator;
 @synthesize popupViewController = _popupViewController;
-@synthesize positioner = _positioner;
 @synthesize dispatcher = _dispatcher;
 
 #pragma mark - Public
@@ -69,10 +68,10 @@
   BOOL isIncognito = self.browserState->IsOffTheRecord();
   self.mediator.incognito = isIncognito;
   self.mediator.consumer = self.popupViewController;
-    self.mediator.presenter = [[OmniboxPopupPresenter alloc]
-        initWithPopupPositioner:self.positioner
-            popupViewController:self.popupViewController
-                      incognito:isIncognito];
+  self.mediator.presenter = [[OmniboxPopupPresenter alloc]
+      initWithPopupPresenterDelegate:self.presenterDelegate
+                 popupViewController:self.popupViewController
+                           incognito:isIncognito];
   self.popupViewController.imageRetriever = self.mediator;
   self.popupViewController.delegate = self.mediator;
   [self.dispatcher

ios/chrome/browser/ui/omnibox/popup/omnibox_popup_positioner.h

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_POPUP_POSITIONER_H_
-#define IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_POPUP_POSITIONER_H_
-
-#import <UIKit/UIKit.h>
-
-// TODO(crbug.com/809785): Rename this or merge it into another protocol.
-@protocol OmniboxPopupPositioner
-
-// View to which the popup view should be added as subview.
-- (UIView*)popupParentView;
-
-// The view controller that will parent the popup.
-- (UIViewController*)popupParentViewController;
-
-@end
-
-#endif  // IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_POPUP_POSITIONER_H_

ios/chrome/browser/ui/omnibox/popup/omnibox_popup_presenter.h

@@ -7,9 +7,27 @@
 
 #import <UIKit/UIKit.h>
 
-@protocol OmniboxPopupPositioner;
+@class OmniboxPopupPresenter;
+@protocol OmniboxPopupPresenterDelegate
+
+// View to which the popup view should be added as subview.
+- (UIView*)popupParentViewForPresenter:(OmniboxPopupPresenter*)presenter;
+
+// The view controller that will parent the popup.
+- (UIViewController*)popupParentViewControllerForPresenter:
+    (OmniboxPopupPresenter*)presenter;
+
+// Alert the delegate that the popup opened.
+- (void)popupDidOpenForPresenter:(OmniboxPopupPresenter*)presenter;
+
+// Alert the delegate that the popup closed.
+- (void)popupDidCloseForPresenter:(OmniboxPopupPresenter*)presenter;
+
+@end
 
 // The UI Refresh implementation of the popup presenter.
+// TODO(crbug.com/936833): This class should be refactored to handle a nil
+// delegate.
 @interface OmniboxPopupPresenter : NSObject
 
 // Updates appearance depending on the content size of the presented view
@@ -19,9 +37,10 @@
 // Call this to hide the popup with animation.
 - (void)animateCollapse;
 
-- (instancetype)initWithPopupPositioner:(id<OmniboxPopupPositioner>)positioner
-                    popupViewController:(UIViewController*)viewController
-                              incognito:(BOOL)incognito;
+- (instancetype)initWithPopupPresenterDelegate:
+                    (id<OmniboxPopupPresenterDelegate>)presenterDelegate
+                           popupViewController:(UIViewController*)viewController
+                                     incognito:(BOOL)incognito;
 
 @end
 

ios/chrome/browser/ui/omnibox/popup/omnibox_popup_presenter.mm

@@ -4,7 +4,6 @@
 
 #import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_presenter.h"
 
-#import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_positioner.h"
 #import "ios/chrome/browser/ui/toolbar/buttons/toolbar_configuration.h"
 #import "ios/chrome/browser/ui/toolbar/public/features.h"
 #import "ios/chrome/browser/ui/util/named_guide.h"
@@ -27,7 +26,7 @@ const CGFloat kVerticalOffset = 6;
 // Constraint for the bottom anchor of the popup.
 @property(nonatomic, strong) NSLayoutConstraint* bottomConstraint;
 
-@property(nonatomic, weak) id<OmniboxPopupPositioner> positioner;
+@property(nonatomic, weak) id<OmniboxPopupPresenterDelegate> delegate;
 @property(nonatomic, weak) UIViewController* viewController;
 @property(nonatomic, strong) UIView* popupContainerView;
 @property(nonatomic) UIViewPropertyAnimator* animator;
@@ -35,12 +34,13 @@ const CGFloat kVerticalOffset = 6;
 
 @implementation OmniboxPopupPresenter
 
-- (instancetype)initWithPopupPositioner:(id<OmniboxPopupPositioner>)positioner
-                    popupViewController:(UIViewController*)viewController
-                              incognito:(BOOL)incognito {
+- (instancetype)initWithPopupPresenterDelegate:
+                    (id<OmniboxPopupPresenterDelegate>)delegate
+                           popupViewController:(UIViewController*)viewController
+                                     incognito:(BOOL)incognito {
   self = [super init];
   if (self) {
-    _positioner = positioner;
+    _delegate = delegate;
     _viewController = viewController;
 
     // Popup uses same colors as the toolbar, so the ToolbarConfiguration is
@@ -72,9 +72,10 @@ const CGFloat kVerticalOffset = 6;
 - (void)updateHeightAndAnimateAppearanceIfNecessary {
   UIView* popup = self.popupContainerView;
   if (!popup.superview) {
-    UIViewController* parentVC = [self.positioner popupParentViewController];
+    UIViewController* parentVC =
+        [self.delegate popupParentViewControllerForPresenter:self];
     [parentVC addChildViewController:self.viewController];
-    [[self.positioner popupParentView] addSubview:popup];
+    [[self.delegate popupParentViewForPresenter:self] addSubview:popup];
     [self.viewController didMoveToParentViewController:parentVC];
 
     [self initialLayout];
@@ -84,6 +85,8 @@ const CGFloat kVerticalOffset = 6;
     self.bottomConstraint.active = YES;
   }
 
+  [self.delegate popupDidOpenForPresenter:self];
+
   [self.animator stopAnimation:YES];
 
   if (popup.bounds.size.height == 0) {
@@ -99,6 +102,7 @@ const CGFloat kVerticalOffset = 6;
 }
 
 - (void)animateCollapse {
+  [self.delegate popupDidCloseForPresenter:self];
   UIView* retainedPopupView = self.popupContainerView;
   UIViewController* retainedViewController = self.viewController;
   if (!IsIPadIdiom()) {

ios/chrome/browser/ui/toolbar/primary_toolbar_coordinator.mm

@@ -18,7 +18,6 @@
 #import "ios/chrome/browser/ui/location_bar/location_bar_coordinator.h"
 #import "ios/chrome/browser/ui/ntp/ntp_util.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.h"
-#import "ios/chrome/browser/ui/omnibox/popup/omnibox_popup_positioner.h"
 #import "ios/chrome/browser/ui/orchestrator/omnibox_focus_orchestrator.h"
 #import "ios/chrome/browser/ui/toolbar/adaptive_toolbar_coordinator+subclassing.h"
 #import "ios/chrome/browser/ui/toolbar/primary_toolbar_view_controller.h"
@@ -33,8 +32,7 @@
 #error "This file requires ARC support."
 #endif
 
-@interface PrimaryToolbarCoordinator ()<OmniboxPopupPositioner,
-                                        PrimaryToolbarViewControllerDelegate> {
+@interface PrimaryToolbarCoordinator () <PrimaryToolbarViewControllerDelegate> {
   // Observer that updates |toolbarViewController| for fullscreen events.
   std::unique_ptr<FullscreenControllerObserver> _fullscreenObserver;
 }
@@ -55,11 +53,9 @@
 @implementation PrimaryToolbarCoordinator
 
 @dynamic viewController;
-@synthesize started = _started;
+@synthesize popupPresenterDelegate = _popupPresenterDelegate;
 @synthesize commandDispatcher = _commandDispatcher;
 @synthesize delegate = _delegate;
-@synthesize locationBarCoordinator = _locationBarCoordinator;
-@synthesize orchestrator = _orchestrator;
 
 #pragma mark - ChromeCoordinator
 
@@ -192,19 +188,6 @@
   self.viewController.view.hidden = NO;
 }
 
-// TODO(crbug.com/786940): This protocol should move to the ViewController
-// owning the Toolbar. This can wait until the omnibox and toolbar refactoring
-// is more advanced.
-#pragma mark OmniboxPopupPositioner methods.
-
-- (UIView*)popupParentView {
-  return self.viewController.view.superview;
-}
-
-- (UIViewController*)popupParentViewController {
-  return self.viewController.parentViewController;
-}
-
 #pragma mark - Protected override
 
 - (void)updateToolbarForSideSwipeSnapshot:(web::WebState*)webState {
@@ -238,7 +221,8 @@
   self.locationBarCoordinator.commandDispatcher = self.commandDispatcher;
   self.locationBarCoordinator.delegate = self.delegate;
   self.locationBarCoordinator.webStateList = self.webStateList;
-  self.locationBarCoordinator.popupPositioner = self;
+  self.locationBarCoordinator.popupPresenterDelegate =
+      self.popupPresenterDelegate;
   [self.locationBarCoordinator start];
 }
 

ios/chrome/browser/ui/toolbar/public/primary_toolbar_coordinator.h

@@ -11,12 +11,17 @@
 
 @protocol ActivityServicePositioner;
 @class CommandDispatcher;
+@protocol OmniboxPopupPresenterDelegate;
 
 // Protocol defining a primary toolbar, in a paradigm where the toolbar can be
 // split between primary and secondary.
 @protocol PrimaryToolbarCoordinator<FakeboxFocuser,
                                     SideSwipeToolbarSnapshotProviding>
 
+// Defines where the omnibox popup will be positioned.
+@property(nonatomic, weak) id<OmniboxPopupPresenterDelegate>
+    popupPresenterDelegate;
+
 // Command dispatcher.
 @property(nonatomic, strong) CommandDispatcher* commandDispatcher;