Move AutofillPopup from NativeWindow to WebContents (#12514)

atom/browser/api/atom_api_web_contents.cc

@@ -608,6 +608,9 @@ void WebContents::MoveContents(content::WebContents* source,
 
 void WebContents::CloseContents(content::WebContents* source) {
   Emit("close");
+#if defined(TOOLKIT_VIEWS)
+  HideAutofillPopup();
+#endif
   if (managed_web_contents())
     managed_web_contents()->GetView()->SetDelegate(nullptr);
   for (ExtendedWebContentsObserver& observer : observers_)
@@ -960,16 +963,16 @@ void WebContents::DevToolsClosed() {
   Emit("devtools-closed");
 }
 
+#if defined(TOOLKIT_VIEWS)
 void WebContents::ShowAutofillPopup(content::RenderFrameHost* frame_host,
                                     const gfx::RectF& bounds,
                                     const std::vector<base::string16>& values,
                                     const std::vector<base::string16>& labels) {
-  auto relay = NativeWindowRelay::FromWebContents(web_contents());
-  if (relay) {
-    relay->window->ShowAutofillPopup(
-        frame_host, web_contents(), bounds, values, labels);
-  }
+  bool offscreen = IsOffScreen() || (embedder_ && embedder_->IsOffScreen());
+  CommonWebContentsDelegate::ShowAutofillPopup(
+      offscreen, frame_host, bounds, values, labels);
 }
+#endif
 
 bool WebContents::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
@@ -986,15 +989,6 @@ bool WebContents::OnMessageReceived(const IPC::Message& message,
                                     content::RenderFrameHost* frame_host) {
   bool handled = true;
   FrameDispatchHelper helper = {this, frame_host};
-  auto relay = NativeWindowRelay::FromWebContents(web_contents());
-  if (relay) {
-    IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(NativeWindow, message, frame_host)
-      IPC_MESSAGE_FORWARD(AtomAutofillFrameHostMsg_HidePopup,
-                          relay->window.get(), NativeWindow::HideAutofillPopup)
-      IPC_MESSAGE_UNHANDLED(handled = false)
-    IPC_END_MESSAGE_MAP()
-  }
-
   IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(WebContents, message, frame_host)
     IPC_MESSAGE_HANDLER(AtomFrameHostMsg_Message, OnRendererMessage)
     IPC_MESSAGE_FORWARD_DELAY_REPLY(AtomFrameHostMsg_Message_Sync, &helper,
@@ -1004,7 +998,10 @@ bool WebContents::OnMessageReceived(const IPC::Message& message,
         FrameDispatchHelper::OnSetTemporaryZoomLevel)
     IPC_MESSAGE_FORWARD_DELAY_REPLY(AtomFrameHostMsg_GetZoomLevel, &helper,
                                     FrameDispatchHelper::OnGetZoomLevel)
+#if defined(TOOLKIT_VIEWS)
     IPC_MESSAGE_HANDLER(AtomAutofillFrameHostMsg_ShowPopup, ShowAutofillPopup)
+    IPC_MESSAGE_HANDLER(AtomAutofillFrameHostMsg_HidePopup, HideAutofillPopup)
+#endif
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 

atom/browser/api/atom_api_web_contents.h

@@ -384,10 +384,12 @@ class WebContents : public mate::TrackableObject<WebContents>,
   void DevToolsOpened() override;
   void DevToolsClosed() override;
 
+#if defined(TOOLKIT_VIEWS)
   void ShowAutofillPopup(content::RenderFrameHost* frame_host,
                          const gfx::RectF& bounds,
                          const std::vector<base::string16>& values,
                          const std::vector<base::string16>& labels);
+#endif
 
  private:
   struct FrameDispatchHelper;

atom/browser/common_web_contents_delegate.cc

@@ -189,6 +189,9 @@ void CommonWebContentsDelegate::SetOwnerWindow(
   auto relay = base::MakeUnique<NativeWindowRelay>(owner_window_);
   auto relay_key = relay->key;
   if (owner_window) {
+#if defined(TOOLKIT_VIEWS)
+    autofill_popup_.reset(new AutofillPopup());
+#endif
     web_contents->SetUserData(relay_key, std::move(relay));
   } else {
     web_contents->RemoveUserData(relay_key);

atom/browser/common_web_contents_delegate.h

@@ -15,6 +15,10 @@
 #include "brightray/browser/inspectable_web_contents_view_delegate.h"
 #include "content/public/browser/web_contents_delegate.h"
 
+#if defined(TOOLKIT_VIEWS)
+#include "atom/browser/ui/autofill_popup.h"
+#endif
+
 using brightray::DevToolsFileSystemIndexer;
 
 namespace atom {
@@ -86,6 +90,17 @@ class CommonWebContentsDelegate
       content::WebContents* source,
       const content::NativeWebKeyboardEvent& event) override;
 
+  // Autofill related events.
+#if defined(TOOLKIT_VIEWS)
+  void ShowAutofillPopup(
+    bool offscreen,
+    content::RenderFrameHost* frame_host,
+    const gfx::RectF& bounds,
+    const std::vector<base::string16>& values,
+    const std::vector<base::string16>& labels);
+  void HideAutofillPopup();
+#endif
+
   // brightray::InspectableWebContentsDelegate:
   void DevToolsSaveToFile(const std::string& url,
                           const std::string& content,
@@ -122,7 +137,7 @@ class CommonWebContentsDelegate
   // Callback for when DevToolsAppendToFile has completed.
   void OnDevToolsAppendToFile(const std::string& url);
 
-  //
+  // DevTools index event callbacks.
   void OnDevToolsIndexingWorkCalculated(int request_id,
                                         const std::string& file_system_path,
                                         int total_work);
@@ -150,7 +165,12 @@ class CommonWebContentsDelegate
   // Whether window is fullscreened by window api.
   bool native_fullscreen_;
 
+  // UI related helper classes.
+#if defined(TOOLKIT_VIEWS)
+  std::unique_ptr<AutofillPopup> autofill_popup_;
+#endif
   std::unique_ptr<WebDialogHelper> web_dialog_helper_;
+
   scoped_refptr<DevToolsFileSystemIndexer> devtools_file_system_indexer_;
 
   // Make sure BrowserContext is alwasys destroyed after WebContents.

atom/browser/common_web_contents_delegate_views.cc

@@ -27,6 +27,26 @@ void CommonWebContentsDelegate::HandleKeyboardEvent(
     owner_window()->HandleKeyboardEvent(source, event);
 }
 
+void CommonWebContentsDelegate::ShowAutofillPopup(
+    bool offscreen,
+    content::RenderFrameHost* frame_host,
+    const gfx::RectF& bounds,
+    const std::vector<base::string16>& values,
+    const std::vector<base::string16>& labels) {
+  if (!owner_window())
+    return;
+
+  auto* window = static_cast<NativeWindowViews*>(owner_window());
+  autofill_popup_->CreateView(
+      frame_host, offscreen, window->web_view(), bounds);
+  autofill_popup_->SetItems(values, labels);
+}
+
+void CommonWebContentsDelegate::HideAutofillPopup() {
+  if (autofill_popup_)
+    autofill_popup_->Hide();
+}
+
 gfx::ImageSkia CommonWebContentsDelegate::GetDevToolsWindowIcon() {
   if (!owner_window())
     return gfx::ImageSkia();

atom/browser/native_window.h

@@ -218,13 +218,6 @@ class NativeWindow : public base::SupportsUserData {
   virtual void HandleKeyboardEvent(
       content::WebContents*,
       const content::NativeWebKeyboardEvent& event) {}
-  virtual void ShowAutofillPopup(
-    content::RenderFrameHost* frame_host,
-    content::WebContents* web_contents,
-    const gfx::RectF& bounds,
-    const std::vector<base::string16>& values,
-    const std::vector<base::string16>& labels) {}
-  virtual void HideAutofillPopup(content::RenderFrameHost* frame_host) {}
 
   // Public API used by platform-dependent delegates and observers to send UI
   // related notifications.

atom/browser/native_window_views.cc

@@ -327,8 +327,6 @@ NativeWindowViews::NativeWindowViews(
   window_->CenterWindow(size);
   Layout();
 
-  autofill_popup_.reset(new AutofillPopup(GetNativeView()));
-
 #if defined(OS_WIN)
   // Save initial window state.
   if (fullscreen)
@@ -1370,49 +1368,6 @@ void NativeWindowViews::HandleKeyboardEvent(
   }
 }
 
-void NativeWindowViews::ShowAutofillPopup(
-    content::RenderFrameHost* frame_host,
-    content::WebContents* web_contents,
-    const gfx::RectF& bounds,
-    const std::vector<base::string16>& values,
-    const std::vector<base::string16>& labels) {
-  bool is_offsceen = false;
-  bool is_embedder_offscreen = false;
-
-  auto* web_preferences = WebContentsPreferences::From(web_contents);
-  if (web_preferences) {
-    web_preferences->dict()->GetBoolean("offscreen", &is_offsceen);
-    int guest_instance_id = 0;
-    web_preferences->dict()->GetInteger(options::kGuestInstanceID,
-                                        &guest_instance_id);
-
-    if (guest_instance_id) {
-      auto* manager = WebViewManager::GetWebViewManager(web_contents);
-      if (manager) {
-        auto* embedder = manager->GetEmbedder(guest_instance_id);
-        if (embedder) {
-          auto* embedder_prefs = WebContentsPreferences::From(embedder);
-          is_embedder_offscreen = embedder_prefs &&
-                                  embedder_prefs->IsEnabled("offscreen");
-        }
-      }
-    }
-  }
-
-  autofill_popup_->CreateView(
-      frame_host,
-      is_offsceen || is_embedder_offscreen,
-      widget(),
-      bounds);
-  autofill_popup_->SetItems(values, labels);
-  autofill_popup_->UpdatePopupBounds(menu_bar_visible_ ? 0 : kMenuBarHeight);
-}
-
-void NativeWindowViews::HideAutofillPopup(
-    content::RenderFrameHost* frame_host) {
-  autofill_popup_->Hide();
-}
-
 void NativeWindowViews::Layout() {
   const auto size = GetContentsBounds().size();
   const auto menu_bar_bounds =
@@ -1427,9 +1382,6 @@ void NativeWindowViews::Layout() {
         gfx::Rect(0, menu_bar_bounds.height(), size.width(),
                   size.height() - menu_bar_bounds.height()));
   }
-
-  if (autofill_popup_.get())
-    autofill_popup_->UpdatePopupBounds(menu_bar_visible_ ? 0 : kMenuBarHeight);
 }
 
 gfx::Size NativeWindowViews::GetMinimumSize() const {

atom/browser/native_window_views.h

@@ -12,7 +12,6 @@
 #include <vector>
 
 #include "atom/browser/ui/accelerator_util.h"
-#include "atom/browser/ui/autofill_popup.h"
 #include "ui/views/widget/widget_delegate.h"
 #include "ui/views/widget/widget_observer.h"
 
@@ -143,6 +142,7 @@ class NativeWindowViews : public NativeWindow,
   void SetEnabled(bool enable) override;
 
   views::Widget* widget() const { return window_.get(); }
+  views::View* web_view() const { return web_view_; }
   SkRegion* draggable_region() const { return draggable_region_.get(); }
 
 #if defined(OS_WIN)
@@ -195,13 +195,6 @@ class NativeWindowViews : public NativeWindow,
   void HandleKeyboardEvent(
       content::WebContents*,
       const content::NativeWebKeyboardEvent& event) override;
-  void ShowAutofillPopup(
-    content::RenderFrameHost* frame_host,
-    content::WebContents* web_contents,
-    const gfx::RectF& bounds,
-    const std::vector<base::string16>& values,
-    const std::vector<base::string16>& labels) override;
-  void HideAutofillPopup(content::RenderFrameHost* frame_host) override;
 
   // views::View:
   void Layout() override;
@@ -219,8 +212,6 @@ class NativeWindowViews : public NativeWindow,
   views::View* web_view_;  // Managed by inspectable_web_contents_.
   views::View* focused_view_;  // The view should be focused by default.
 
-  std::unique_ptr<AutofillPopup> autofill_popup_;
-
   std::unique_ptr<MenuBar> menu_bar_;
   bool menu_bar_autohide_;
   bool menu_bar_visible_;

atom/browser/ui/autofill_popup.cc

@@ -6,19 +6,22 @@
 #include <utility>
 #include <vector>
 
-#if defined(ENABLE_OSR)
-#include "atom/browser/osr/osr_render_widget_host_view.h"
-#include "atom/browser/osr/osr_view_proxy.h"
-#endif
+#include "atom/browser/native_window_views.h"
 #include "atom/browser/ui/autofill_popup.h"
 #include "atom/common/api/api_messages.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/rect_conversions.h"
 #include "ui/gfx/geometry/vector2d.h"
 #include "ui/gfx/text_utils.h"
 
+#if defined(ENABLE_OSR)
+#include "atom/browser/osr/osr_render_widget_host_view.h"
+#include "atom/browser/osr/osr_view_proxy.h"
+#endif
+
 namespace atom {
 
 namespace {
@@ -95,20 +98,17 @@ std::pair<int, int> CalculatePopupYAndHeight(
   }
 }
 
-display::Display GetDisplayNearestPoint(
-    const gfx::Point& point,
-    gfx::NativeView container_view) {
+display::Display GetDisplayNearestPoint(const gfx::Point& point) {
   return display::Screen::GetScreen()->GetDisplayNearestPoint(point);
 }
 
 }  // namespace
 
-AutofillPopup::AutofillPopup(gfx::NativeView container_view)
-    : container_view_(container_view), view_(nullptr) {
+AutofillPopup::AutofillPopup() {
   bold_font_list_ =
-    gfx::FontList().DeriveWithWeight(gfx::Font::Weight::BOLD);
+      gfx::FontList().DeriveWithWeight(gfx::Font::Weight::BOLD);
   smaller_font_list_ =
-    gfx::FontList().DeriveWithSizeDelta(kSmallerFontSizeDelta);
+      gfx::FontList().DeriveWithSizeDelta(kSmallerFontSizeDelta);
 }
 
 AutofillPopup::~AutofillPopup() {
@@ -118,20 +118,17 @@ AutofillPopup::~AutofillPopup() {
 void AutofillPopup::CreateView(
     content::RenderFrameHost* frame_host,
     bool offscreen,
-    views::Widget* parent_widget,
+    views::View* parent,
     const gfx::RectF& r) {
+  Hide();
+
   frame_host_ = frame_host;
-  gfx::Rect lb(std::floor(r.x()), std::floor(r.y() + r.height()),
-               std::floor(r.width()), std::floor(r.height()));
-  gfx::Point menu_position(lb.origin());
-  popup_bounds_in_view_ = lb;
-  views::View::ConvertPointToScreen(
-    parent_widget->GetContentsView(), &menu_position);
-  popup_bounds_ = gfx::Rect(menu_position, lb.size());
-  element_bounds_ = popup_bounds_;
+  element_bounds_ = gfx::ToEnclosedRect(r);
 
-  Hide();
-  view_ = new AutofillPopupView(this, parent_widget);
+  parent_ = parent;
+  parent_->AddObserver(this);
+
+  view_ = new AutofillPopupView(this, parent->GetWidget());
   view_->Show();
 
 #if defined(ENABLE_OSR)
@@ -145,6 +142,10 @@ void AutofillPopup::CreateView(
 }
 
 void AutofillPopup::Hide() {
+  if (parent_) {
+    parent_->RemoveObserver(this);
+    parent_ = nullptr;
+  }
   if (view_) {
     view_->Hide();
     view_ = nullptr;
@@ -152,28 +153,31 @@ void AutofillPopup::Hide() {
 }
 
 void AutofillPopup::SetItems(const std::vector<base::string16>& values,
-                            const std::vector<base::string16>& labels) {
+                             const std::vector<base::string16>& labels) {
+  DCHECK(view_);
   values_ = values;
   labels_ = labels;
-  if (view_) {
-    view_->OnSuggestionsChanged();
-  }
+  UpdatePopupBounds();
+  view_->OnSuggestionsChanged();
+  if (view_)  // could be hidden after the change
+    view_->DoUpdateBoundsAndRedrawPopup();
 }
 
 void AutofillPopup::AcceptSuggestion(int index) {
   frame_host_->Send(new AtomAutofillFrameMsg_AcceptSuggestion(
-    frame_host_->GetRoutingID(), GetValueAt(index)));
+      frame_host_->GetRoutingID(), GetValueAt(index)));
 }
 
-void AutofillPopup::UpdatePopupBounds(int height_compensation) {
+void AutofillPopup::UpdatePopupBounds() {
+  DCHECK(parent_);
+  gfx::Point origin(element_bounds_.origin());
+  views::View::ConvertPointToScreen(parent_, &origin);
+  gfx::Rect bounds(origin, element_bounds_.size());
+
   int desired_width = GetDesiredPopupWidth();
   int desired_height = GetDesiredPopupHeight();
   bool is_rtl = false;
 
-  gfx::Point origin(element_bounds_.origin().x(),
-                    element_bounds_.origin().y() - height_compensation);
-  gfx::Rect bounds(origin, element_bounds_.size());
-
   gfx::Point top_left_corner_of_popup =
       origin + gfx::Vector2d(bounds.width() - desired_width, -desired_height);
 
@@ -184,9 +188,9 @@ void AutofillPopup::UpdatePopupBounds(int height_compensation) {
       origin + gfx::Vector2d(desired_width, bounds.height() + desired_height);
 
   display::Display top_left_display =
-      GetDisplayNearestPoint(top_left_corner_of_popup, container_view_);
+      GetDisplayNearestPoint(top_left_corner_of_popup);
   display::Display bottom_right_display =
-      GetDisplayNearestPoint(bottom_right_corner_of_popup, container_view_);
+      GetDisplayNearestPoint(bottom_right_corner_of_popup);
 
   std::pair<int, int> popup_x_and_width =
       CalculatePopupXAndWidth(top_left_display, bottom_right_display,
@@ -201,8 +205,15 @@ void AutofillPopup::UpdatePopupBounds(int height_compensation) {
   popup_bounds_in_view_ = gfx::Rect(
       popup_bounds_in_view_.origin(),
       gfx::Size(popup_x_and_width.second, popup_y_and_height.second));
-  if (view_)
-    view_->DoUpdateBoundsAndRedrawPopup();
+}
+
+void AutofillPopup::OnViewBoundsChanged(views::View* view) {
+  UpdatePopupBounds();
+  view_->DoUpdateBoundsAndRedrawPopup();
+}
+
+void AutofillPopup::OnViewIsDeleting(views::View* view) {
+  Hide();
 }
 
 int AutofillPopup::GetDesiredPopupHeight() {

atom/browser/ui/autofill_popup.h

@@ -18,22 +18,28 @@ namespace atom {
 
 class AutofillPopupView;
 
-class AutofillPopup {
+class AutofillPopup : public views::ViewObserver {
  public:
-  explicit AutofillPopup(gfx::NativeView);
+  AutofillPopup();
   ~AutofillPopup();
 
   void CreateView(content::RenderFrameHost* render_frame,
-    bool offscreen, views::Widget* widget, const gfx::RectF& bounds);
+                  bool offscreen,
+                  views::View* parent,
+                  const gfx::RectF& bounds);
   void Hide();
 
   void SetItems(const std::vector<base::string16>& values,
                 const std::vector<base::string16>& labels);
-  void UpdatePopupBounds(int height_compensation);
+  void UpdatePopupBounds();
 
  private:
   friend class AutofillPopupView;
 
+  // views::ViewObserver:
+  void OnViewBoundsChanged(views::View* view) override;
+  void OnViewIsDeleting(views::View* view) override;
+
   void AcceptSuggestion(int index);
 
   int GetDesiredPopupHeight();
@@ -48,9 +54,6 @@ class AutofillPopup {
   base::string16 GetLabelAt(int i);
   int LineFromY(int y) const;
 
-  // The native view that contains this
-  gfx::NativeView container_view_;
-
   int selected_index_;
 
   // Popup location
@@ -70,10 +73,13 @@ class AutofillPopup {
 
   // For sending the accepted suggestion to the render frame that
   // asked to open the popup
-  content::RenderFrameHost* frame_host_;
+  content::RenderFrameHost* frame_host_ = nullptr;
 
   // The popup view. The lifetime is managed by the owning Widget
-  AutofillPopupView* view_;
+  AutofillPopupView* view_ = nullptr;
+
+  // The parent view that the popup view shows on. Weak ref.
+  views::View* parent_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(AutofillPopup);
 };

atom/renderer/renderer_client_base.cc

@@ -148,7 +148,9 @@ void RendererClientBase::RenderThreadStarted() {
 
 void RendererClientBase::RenderFrameCreated(
     content::RenderFrame* render_frame) {
+#if defined(TOOLKIT_VIEWS)
   new AutofillAgent(render_frame);
+#endif
   new PepperHelper(render_frame);
   new ContentSettingsObserver(render_frame);
   new printing::PrintWebViewHelper(render_frame);