[iOS] Prevent empty omnibox popup from opening on incognito pages

Before, the call to [self.omniboxPopupCoordinator openPopup] in the
location bar coordinator would always add the popup, under the
assumption that there would always be at least one suggestion (the
current page's URL). However, on incognito pages (both NTP and
regular pages), this doesn't appear and there are no suggestions, so
the popup appears, but is blank.

Instead of manually ordering the popup to appear or disappear, we can
use the popup's intrinsic content size, which is already how the
popup is being sized on iPad. Thus, when we were ordering the popup
to appear or collapse, we can instead just update the display status.

Bug: 933133
Change-Id: I11d3dff55f1a2f13daad259370e81df6ad7cc55a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1514573
Commit-Queue: Robbie Gibson <rkgibson@google.com>
Reviewed-by: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641681}

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

@@ -298,13 +298,13 @@ const int kLocationAuthorizationStatusCount = 4;
   [[UIMenuController sharedMenuController] setMenuVisible:NO animated:NO];
 
   // When the NTP and fakebox are visible, make the fakebox animates into place
-  // before focusing the omnibox.webState
+  // before focusing the omnibox.
   if (IsVisibleURLNewTabPage([self webState]) &&
       !self.browserState->IsOffTheRecord()) {
     [self.viewController.dispatcher focusFakebox];
   } else {
     [self.omniboxCoordinator focusOmnibox];
-    [self.omniboxPopupCoordinator openPopup];
+    [self.omniboxPopupCoordinator presentShortcutsIfNecessary];
   }
 }
 
@@ -314,7 +314,7 @@ const int kLocationAuthorizationStatusCount = 4;
   }
   self.isCancellingOmniboxEdit = YES;
   [self.omniboxCoordinator endEditing];
-  [self.omniboxPopupCoordinator closePopup];
+  [self.omniboxPopupCoordinator dismissShortcuts];
   self.isCancellingOmniboxEdit = NO;
 }
 

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

@@ -42,11 +42,12 @@ class WebStateList;
 - (void)start;
 - (void)stop;
 
-// Opens the popup immediately. It's auto-sized to fit the suggestions.
-- (void)openPopup;
+// Presents the shortcuts feature if the current page allows for it and update
+// the popup.
+- (void)presentShortcutsIfNecessary;
 
-// Closes the popup immediately.
-- (void)closePopup;
+// Dismisses the shortcuts feature and update the popup.
+- (void)dismissShortcuts;
 
 @end
 

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

@@ -93,7 +93,7 @@
   return self.mediator.isOpen;
 }
 
-- (void)openPopup {
+- (void)presentShortcutsIfNecessary {
   // Initialize the shortcuts feature when necessary.
   if (base::FeatureList::IsEnabled(
           omnibox::kOmniboxPopupShortcutIconsInZeroState) &&
@@ -118,14 +118,14 @@
     self.popupViewController.shortcutsEnabled = YES;
   }
 
-  [self.mediator.presenter updateHeight];
-  self.mediator.open = YES;
+  [self.mediator.presenter updatePopup];
+  self.mediator.open = self.mediator.presenter.isOpen;
 }
 
-- (void)closePopup {
-  self.mediator.open = NO;
+- (void)dismissShortcuts {
   self.popupViewController.shortcutsEnabled = NO;
-  [self.mediator.presenter collapse];
+  [self.mediator.presenter updatePopup];
+  self.mediator.open = self.mediator.presenter.isOpen;
 }
 
 #pragma mark - Property accessor

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

@@ -58,14 +58,6 @@
   self.hasResults = !_currentResult.empty();
 
   [self.consumer updateMatches:[self wrappedMatches] withAnimation:animation];
-
-  BOOL shortcutsEnabled = base::FeatureList::IsEnabled(
-      omnibox::kOmniboxPopupShortcutIconsInZeroState);
-  BOOL isNTP = IsVisibleURLNewTabPage(self.webStateList->GetActiveWebState());
-
-  if (!self.hasResults && (!shortcutsEnabled || isNTP)) {
-    [self.presenter collapse];
-  }
 }
 
 - (NSArray<id<AutocompleteSuggestion>>*)wrappedMatches {
@@ -98,9 +90,7 @@
   }
   self.open = !result.empty();
 
-  if (self.open) {
-    [self.presenter updateHeight];
-  }
+  [self.presenter updatePopup];
 }
 
 - (void)setTextAlignment:(NSTextAlignment)alignment {

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

@@ -8,6 +8,7 @@
 #import <UIKit/UIKit.h>
 
 @class OmniboxPopupPresenter;
+
 @protocol OmniboxPopupPresenterDelegate
 
 // View to which the popup view should be added as subview.
@@ -30,11 +31,12 @@
 // delegate.
 @interface OmniboxPopupPresenter : NSObject
 
-// Updates appearance depending on the content size of the presented view
-// controller by changing the visible height of the popup.
-- (void)updateHeight;
-// Hides the popup.
-- (void)collapse;
+// Whether the popup is open
+@property(nonatomic, assign, getter=isOpen) BOOL open;
+
+// Uses the popup's intrinsic content size to add or remove the popup view
+// if necessary.
+- (void)updatePopup;
 
 - (instancetype)initWithPopupPresenterDelegate:
                     (id<OmniboxPopupPresenterDelegate>)presenterDelegate

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

@@ -66,35 +66,41 @@ const CGFloat kVerticalOffset = 6;
   return self;
 }
 
-- (void)updateHeight {
-  UIView* popup = self.popupContainerView;
-  if (!popup.superview) {
+- (void)updatePopup {
+  BOOL popupSizeIsZero = CGSizeEqualToSize(
+      [self.viewController.view intrinsicContentSize], CGSizeZero);
+  BOOL popupIsOnscreen = self.popupContainerView.superview != nil;
+  if (popupSizeIsZero && popupIsOnscreen) {
+    // If intrinsic size is 0 and popup is onscreen, we want to remove the
+    // popup view.
+    if (!IsIPadIdiom()) {
+      self.bottomConstraint.active = NO;
+    }
+
+    [self.viewController willMoveToParentViewController:nil];
+    [self.popupContainerView removeFromSuperview];
+    [self.viewController removeFromParentViewController];
+
+    self.open = NO;
+    [self.delegate popupDidCloseForPresenter:self];
+  } else if (!popupSizeIsZero && !popupIsOnscreen) {
+    // If intrinsic size is nonzero and popup is offscreen, we want to add it.
     UIViewController* parentVC =
         [self.delegate popupParentViewControllerForPresenter:self];
     [parentVC addChildViewController:self.viewController];
-    [[self.delegate popupParentViewForPresenter:self] addSubview:popup];
+    [[self.delegate popupParentViewForPresenter:self]
+        addSubview:self.popupContainerView];
     [self.viewController didMoveToParentViewController:parentVC];
 
     [self initialLayout];
-  }
 
-  if (!IsIPadIdiom()) {
-    self.bottomConstraint.active = YES;
-  }
-
-  [self.delegate popupDidOpenForPresenter:self];
-}
+    if (!IsIPadIdiom()) {
+      self.bottomConstraint.active = YES;
+    }
 
-- (void)collapse {
-  if (!IsIPadIdiom()) {
-    self.bottomConstraint.active = NO;
+    self.open = YES;
+    [self.delegate popupDidOpenForPresenter:self];
   }
-
-  [self.viewController willMoveToParentViewController:nil];
-  [self.popupContainerView removeFromSuperview];
-  [self.viewController removeFromParentViewController];
-
-  [self.delegate popupDidCloseForPresenter:self];
 }
 
 #pragma mark - Private

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

@@ -87,8 +87,11 @@
     }
   }
 
-  return CGSizeMake(0,
-                    height + self.contentInset.top + self.contentInset.bottom);
+  CGFloat viewHeight =
+      (height == 0) ? 0
+                    : height + self.contentInset.top + self.contentInset.bottom;
+
+  return CGSizeMake(0, viewHeight);
 }
 
 @end