Instantiate popup separately from Omnibox.

This allows instantiating OmniboxPopupViewIOS separately, as opposed to
it being a part of OmniboxViewIOS.

Bug: 
Change-Id: I8e26e3d73e4c85b89448c4812eccf3002705a702
Reviewed-on: https://chromium-review.googlesource.com/692015
Commit-Queue: Stepan Khapugin <stkhapugin@chromium.org>
Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506379}

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

@@ -48,6 +48,7 @@ source_set("omnibox_internal") {
     "omnibox_popup_positioner.h",
     "omnibox_popup_presenter.h",
     "omnibox_popup_presenter.mm",
+    "omnibox_popup_provider.h",
     "omnibox_popup_row.h",
     "omnibox_popup_row.mm",
     "omnibox_popup_view_controller.h",

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

@@ -31,17 +31,21 @@ class OmniboxViewIOS;
 @protocol OmniboxPopupPositioner;
 @class OmniboxTextFieldIOS;
 class ToolbarModel;
+class OmniboxPopupViewIOS;
 
 // Concrete implementation of the LocationBarController interface.
 class LocationBarControllerImpl : public LocationBarController {
  public:
   LocationBarControllerImpl(OmniboxTextFieldIOS* field,
                             ios::ChromeBrowserState* browser_state,
-                            id<OmniboxPopupPositioner> positioner,
                             id<LocationBarDelegate> delegate,
                             id<BrowserCommands> dispatcher);
   ~LocationBarControllerImpl() override;
 
+  // Creates a popup view and wires it to |edit_view_|.
+  std::unique_ptr<OmniboxPopupViewIOS> CreatePopupView(
+      id<OmniboxPopupPositioner> positioner);
+
   // OmniboxEditController implementation
   void OnAutocompleteAccept(const GURL& url,
                             WindowOpenDisposition disposition,
@@ -84,6 +88,7 @@ class LocationBarControllerImpl : public LocationBarController {
   bool show_hint_text_;
   __strong UIButton* clear_text_button_;
   std::unique_ptr<OmniboxViewIOS> edit_view_;
+
   __strong OmniboxClearButtonBridge* clear_button_bridge_;
   // A bridge from a UIControl action to the dispatcher to display a page
   // info popup.

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

@@ -21,6 +21,7 @@
 #include "ios/chrome/browser/experimental_flags.h"
 #import "ios/chrome/browser/ui/commands/browser_commands.h"
 #include "ios/chrome/browser/ui/omnibox/location_bar_delegate.h"
+#include "ios/chrome/browser/ui/omnibox/omnibox_popup_view_ios.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.h"
 #include "ios/chrome/browser/ui/omnibox/omnibox_view_ios.h"
 #include "ios/chrome/browser/ui/ui_util.h"
@@ -142,13 +143,9 @@ bool IsCurrentPageOffline(web::WebState* webState) {
 LocationBarControllerImpl::LocationBarControllerImpl(
     OmniboxTextFieldIOS* field,
     ios::ChromeBrowserState* browser_state,
-    id<OmniboxPopupPositioner> positioner,
     id<LocationBarDelegate> delegate,
     id<BrowserCommands> dispatcher)
-    : edit_view_(base::MakeUnique<OmniboxViewIOS>(field,
-                                                  this,
-                                                  browser_state,
-                                                  positioner)),
+    : edit_view_(base::MakeUnique<OmniboxViewIOS>(field, this, browser_state)),
       field_(field),
       delegate_(delegate),
       dispatcher_(dispatcher) {
@@ -161,6 +158,19 @@ LocationBarControllerImpl::LocationBarControllerImpl(
 
 LocationBarControllerImpl::~LocationBarControllerImpl() {}
 
+std::unique_ptr<OmniboxPopupViewIOS> LocationBarControllerImpl::CreatePopupView(
+    id<OmniboxPopupPositioner> positioner) {
+  std::unique_ptr<OmniboxPopupViewIOS> popup_view =
+      base::MakeUnique<OmniboxPopupViewIOS>(edit_view_->browser_state(),
+                                            edit_view_->model(),
+                                            edit_view_.get(), positioner);
+
+  edit_view_->model()->set_popup_model(popup_view->model());
+  edit_view_->SetPopupProvider(popup_view.get());
+
+  return popup_view;
+}
+
 void LocationBarControllerImpl::HideKeyboardAndEndEditing() {
   edit_view_->HideKeyboardAndEndEditing();
 }

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

+// Copyright 2017 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_PROVIDER_H_
+#define IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_POPUP_PROVIDER_H_
+
+#import <UIKit/UIKit.h>
+
+// An interface for a provider of an omnibox popup. Allows to get information
+// about the state of and configure the popup.
+class OmniboxPopupProvider {
+ public:
+  virtual bool IsPopupOpen() = 0;
+  virtual void SetTextAlignment(NSTextAlignment alignment) = 0;
+};
+
+#endif  // IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_POPUP_PROVIDER_H_

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

@@ -13,6 +13,7 @@
 #include "base/strings/string16.h"
 #include "components/omnibox/browser/omnibox_popup_view.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_popup_mediator.h"
+#include "ios/chrome/browser/ui/omnibox/omnibox_popup_provider.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_popup_view_controller.h"
 
 class OmniboxEditModel;
@@ -29,7 +30,8 @@ class ChromeBrowserState;
 
 // iOS implementation of AutocompletePopupView.
 class OmniboxPopupViewIOS : public OmniboxPopupView,
-                            public OmniboxPopupMediatorDelegate {
+                            public OmniboxPopupMediatorDelegate,
+                            public OmniboxPopupProvider {
  public:
   OmniboxPopupViewIOS(ios::ChromeBrowserState* browser_state,
                       OmniboxEditModel* edit_model,
@@ -37,6 +39,9 @@ class OmniboxPopupViewIOS : public OmniboxPopupView,
                       id<OmniboxPopupPositioner> positioner);
   ~OmniboxPopupViewIOS() override;
 
+  // Popup model used for this.
+  OmniboxPopupModel* model() const;
+
   // AutocompletePopupView implementation.
   bool IsOpen() const override;
   void InvalidateLine(size_t line) override {}
@@ -48,7 +53,10 @@ class OmniboxPopupViewIOS : public OmniboxPopupView,
   void OnDragCanceled() override {}
 
   void UpdateEditViewIcon();
-  void SetTextAlignment(NSTextAlignment alignment);
+
+  // OmniboxPopupProvider implemetation.
+  void SetTextAlignment(NSTextAlignment alignment) override;
+  bool IsPopupOpen() override;
 
   // OmniboxPopupViewControllerDelegate implementation.
   bool IsStarredMatch(const AutocompleteMatch& match) const override;

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

@@ -113,14 +113,24 @@ gfx::Rect OmniboxPopupViewIOS::GetTargetBounds() {
   return gfx::Rect();
 }
 
-void OmniboxPopupViewIOS::SetTextAlignment(NSTextAlignment alignment) {
-  [popup_controller_ setTextAlignment:alignment];
+bool OmniboxPopupViewIOS::IsOpen() const {
+  return is_open_;
 }
 
-bool OmniboxPopupViewIOS::IsOpen() const {
+OmniboxPopupModel* OmniboxPopupViewIOS::model() const {
+  return model_.get();
+}
+
+#pragma mark - OmniboxPopupProvider
+
+bool OmniboxPopupViewIOS::IsPopupOpen() {
   return is_open_;
 }
 
+void OmniboxPopupViewIOS::SetTextAlignment(NSTextAlignment alignment) {
+  [popup_controller_ setTextAlignment:alignment];
+}
+
 #pragma mark - OmniboxPopupViewControllerDelegate
 
 bool OmniboxPopupViewIOS::IsStarredMatch(const AutocompleteMatch& match) const {

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

@@ -11,6 +11,7 @@
 #include "base/mac/scoped_nsobject.h"
 #include "components/omnibox/browser/omnibox_view.h"
 #include "components/toolbar/toolbar_model.h"
+#include "ios/chrome/browser/ui/omnibox/omnibox_popup_provider.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_popup_view_suggestions_delegate.h"
 #import "ios/chrome/browser/ui/omnibox/omnibox_text_field_ios.h"
 
@@ -20,9 +21,7 @@ class AutocompleteResult;
 class GURL;
 @class OmniboxTextFieldIOS;
 @class OmniboxTextFieldPasteDelegate;
-@protocol OmniboxPopupPositioner;
 class WebOmniboxEditController;
-class OmniboxPopupViewIOS;
 
 namespace ios {
 class ChromeBrowserState;
@@ -36,10 +35,13 @@ class OmniboxViewIOS : public OmniboxView,
   // Retains |field|.
   OmniboxViewIOS(OmniboxTextFieldIOS* field,
                  WebOmniboxEditController* controller,
-                 ios::ChromeBrowserState* browser_state,
-                 id<OmniboxPopupPositioner> positioner);
+                 ios::ChromeBrowserState* browser_state);
   ~OmniboxViewIOS() override;
 
+  void SetPopupProvider(OmniboxPopupProvider* provider) {
+    popup_provider_ = provider;
+  };
+
   // Returns a color representing |security_level|, adjusted based on whether
   // the browser is in Incognito mode.
   static UIColor* GetSecureTextColor(
@@ -170,7 +172,6 @@ class OmniboxViewIOS : public OmniboxView,
   base::scoped_nsobject<OmniboxTextFieldIOS> field_;
   base::scoped_nsobject<OmniboxTextFieldPasteDelegate> paste_delegate_;
   WebOmniboxEditController* controller_;  // weak, owns us
-  std::unique_ptr<OmniboxPopupViewIOS> popup_view_;
 
   State state_before_change_;
   base::scoped_nsobject<NSString> marked_text_before_change_;
@@ -193,6 +194,8 @@ class OmniboxViewIOS : public OmniboxView,
   // Temporary pointer to the attributed display string, stored as color and
   // other emphasis attributes are applied by the superclass.
   NSMutableAttributedString* attributing_display_string_;
+
+  OmniboxPopupProvider* popup_provider_;  // weak
 };
 
 #endif  // IOS_CHROME_BROWSER_UI_OMNIBOX_OMNIBOX_VIEW_IOS_H_

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

@@ -24,7 +24,6 @@
 #include "ios/chrome/browser/autocomplete/autocomplete_scheme_classifier_impl.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.h"
-#include "ios/chrome/browser/ui/omnibox/omnibox_popup_view_ios.h"
 #include "ios/chrome/browser/ui/omnibox/omnibox_text_field_paste_delegate.h"
 #include "ios/chrome/browser/ui/omnibox/omnibox_util.h"
 #include "ios/chrome/browser/ui/omnibox/web_omnibox_edit_controller.h"
@@ -161,8 +160,7 @@ UIColor* IncognitoSecureTextColor() {
 
 OmniboxViewIOS::OmniboxViewIOS(OmniboxTextFieldIOS* field,
                                WebOmniboxEditController* controller,
-                               ios::ChromeBrowserState* browser_state,
-                               id<OmniboxPopupPositioner> positioner)
+                               ios::ChromeBrowserState* browser_state)
     : OmniboxView(
           controller,
           base::MakeUnique<ChromeOmniboxClientIOS>(controller, browser_state)),
@@ -170,10 +168,9 @@ OmniboxViewIOS::OmniboxViewIOS(OmniboxTextFieldIOS* field,
       field_(field),
       controller_(controller),
       ignore_popup_updates_(false),
-      attributing_display_string_(nil) {
+      attributing_display_string_(nil),
+      popup_provider_(nullptr) {
   DCHECK(field_);
-  popup_view_ = base::MakeUnique<OmniboxPopupViewIOS>(
-      this->browser_state(), model(), this, positioner);
   field_delegate_.reset(
       [[AutocompleteTextFieldDelegate alloc] initWithEditView:this]);
 
@@ -196,9 +193,6 @@ OmniboxViewIOS::~OmniboxViewIOS() {
   [field_ removeTarget:field_delegate_
                 action:@selector(textFieldDidChange:)
       forControlEvents:UIControlEventEditingChanged];
-
-  // Destroy the model, in case it tries to call back into us when destroyed.
-  popup_view_.reset();
 }
 
 void OmniboxViewIOS::OpenMatch(const AutocompleteMatch& match,
@@ -262,7 +256,8 @@ void OmniboxViewIOS::UpdatePopup() {
       NSMaxRange(current_selection_) != [[field_ text] length];
   model()->StartAutocomplete(current_selection_.length != 0,
                              prevent_inline_autocomplete);
-  popup_view_->SetTextAlignment([field_ bestTextAlignment]);
+  DCHECK(popup_provider_);
+  popup_provider_->SetTextAlignment([field_ bestTextAlignment]);
 }
 
 void OmniboxViewIOS::OnTemporaryTextMaybeChanged(
@@ -354,7 +349,8 @@ void OmniboxViewIOS::OnDidBeginEditing() {
   // If Open from Clipboard offers a suggestion, the popup may be opened when
   // |OnSetFocus| is called on the model. The state of the popup is saved early
   // to ignore that case.
-  bool popup_was_open_before_editing_began = popup_view_->IsOpen();
+  DCHECK(popup_provider_);
+  bool popup_was_open_before_editing_began = popup_provider_->IsPopupOpen();
 
   // Text attributes (e.g. text color) should not be shown while editing, so
   // strip them out by calling setText (as opposed to setAttributedText).
@@ -804,7 +800,8 @@ void OmniboxViewIOS::FocusOmnibox() {
 }
 
 BOOL OmniboxViewIOS::IsPopupOpen() {
-  return popup_view_->IsOpen();
+  DCHECK(popup_provider_);
+  return popup_provider_->IsPopupOpen();
 }
 
 int OmniboxViewIOS::GetIcon(bool offlinePage) const {

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

@@ -47,6 +47,7 @@
 #include "ios/chrome/browser/ui/omnibox/location_bar_controller.h"
 #include "ios/chrome/browser/ui/omnibox/location_bar_controller_impl.h"
 #include "ios/chrome/browser/ui/omnibox/location_bar_delegate.h"
+#include "ios/chrome/browser/ui/omnibox/omnibox_popup_view_ios.h"
 #include "ios/chrome/browser/ui/omnibox/omnibox_view_ios.h"
 #import "ios/chrome/browser/ui/popup_menu/popup_menu_view.h"
 #import "ios/chrome/browser/ui/reversed_animation.h"
@@ -249,7 +250,8 @@ CGRect RectShiftedDownAndResizedForStatusBar(CGRect rect) {
   UIImageView* _incognitoIcon;
   UIView* _clippingView;
 
-  std::unique_ptr<LocationBarController> _locationBar;
+  std::unique_ptr<LocationBarControllerImpl> _locationBar;
+  std::unique_ptr<OmniboxPopupViewIOS> _popupView;
   BOOL _initialLayoutComplete;
   // If |YES|, toolbar is incognito.
   BOOL _incognito;
@@ -621,7 +623,8 @@ CGRect RectShiftedDownAndResizedForStatusBar(CGRect rect) {
                                    UIViewAutoresizingFlexibleBottomMargin];
   [_webToolbar setFrame:[self specificControlsArea]];
   _locationBar = base::MakeUnique<LocationBarControllerImpl>(
-      _omniBox, _browserState, self, self, self.dispatcher);
+      _omniBox, _browserState, self, self.dispatcher);
+  _popupView = _locationBar->CreatePopupView(self);
 
   // Create the determinate progress bar (phone only).
   if (idiom == IPHONE_IDIOM) {
@@ -682,7 +685,8 @@ CGRect RectShiftedDownAndResizedForStatusBar(CGRect rect) {
 
 - (void)browserStateDestroyed {
   // The location bar has a browser state reference, so must be destroyed at
-  // this point.
+  // this point. The popup has to be destroyed before the location bar.
+  _popupView.reset();
   _locationBar.reset();
   _browserState = nullptr;
 }

ios/clean/chrome/browser/ui/omnibox/location_bar_coordinator.mm

@@ -46,8 +46,7 @@
       initWithWebStateList:&(browser->web_state_list())];
   std::unique_ptr<LocationBarController> locationBar =
       base::MakeUnique<LocationBarControllerImpl>(
-          self.viewController.omnibox, browser->browser_state(),
-          nil /* OmniboxPopupPositioner */, self.mediator,
+          self.viewController.omnibox, browser->browser_state(), self.mediator,
           nil /* dispatcher */);
   [self.mediator setLocationBar:std::move(locationBar)];
   [super start];