move popup related code to nativewindow

atom/browser/api/atom_api_web_contents.cc

@@ -17,7 +17,6 @@
 #include "atom/browser/child_web_contents_tracker.h"
 #include "atom/browser/lib/bluetooth_chooser.h"
 #include "atom/browser/native_window.h"
-#include "atom/browser/native_window_views.h"
 #include "atom/browser/net/atom_network_delegate.h"
 #include "atom/browser/osr/osr_output_device.h"
 #include "atom/browser/osr/osr_render_widget_host_view.h"
@@ -84,7 +83,6 @@
 #include "third_party/WebKit/public/web/WebFindOptions.h"
 #include "ui/display/screen.h"
 #include "ui/events/base_event_utils.h"
-#include "ui/gfx/geometry/rect_f.h"
 
 #if !defined(OS_MACOSX)
 #include "ui/aura/window.h"
@@ -443,8 +441,6 @@ void WebContents::InitWithSessionAndOptions(v8::Isolate* isolate,
   registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_PENDING,
                  content::Source<content::NavigationController>(controller));
 
-  autofill_popup_ = new AutofillPopup(web_contents->GetNativeView());
-
   Init(isolate);
   AttachAsUserData(web_contents);
 }
@@ -744,17 +740,6 @@ void WebContents::RenderViewCreated(content::RenderViewHost* render_view_host) {
     impl->disable_hidden_ = !background_throttling_;
 }
 
-void WebContents::RenderFrameCreated(content::RenderFrameHost* host) {
-  Send(new AtomAutofillViewHostMsg_RoutingId(
-    host->GetRoutingID(), routing_id()));
-}
-
-void WebContents::RenderFrameHostChanged(content::RenderFrameHost* old_host,
-    content::RenderFrameHost* new_host) {
-  Send(new AtomAutofillViewHostMsg_RoutingId(
-    new_host->GetRoutingID(), routing_id()));
-}
-
 void WebContents::RenderViewDeleted(content::RenderViewHost* render_view_host) {
   Emit("render-view-deleted", render_view_host->GetProcess()->GetID());
 }
@@ -991,8 +976,23 @@ bool WebContents::OnMessageReceived(const IPC::Message& message) {
                                     OnGetZoomLevel)
     IPC_MESSAGE_HANDLER_CODE(ViewHostMsg_SetCursor, OnCursorChange,
       handled = false)
-    IPC_MESSAGE_HANDLER(AtomAutofillViewMsg_ShowPopup, OnShowAutofillPopup)
+    IPC_MESSAGE_UNHANDLED(handled = false)
-    IPC_MESSAGE_HANDLER(AtomAutofillViewMsg_HidePopup, OnHideAutofillPopup)
+  IPC_END_MESSAGE_MAP()
+
+  return handled;
+}
+
+bool WebContents::OnMessageReceived(const IPC::Message& message,
+    content::RenderFrameHost* frame_host) {
+  bool handled = true;
+  auto relay = NativeWindowRelay::FromWebContents(web_contents());
+  if (!relay)
+    return false;
+  IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(NativeWindow, message, frame_host)
+    IPC_MESSAGE_FORWARD(AtomAutofillFrameHostMsg_ShowPopup,
+      relay->window.get(), NativeWindow::ShowAutofillPopup)
+    IPC_MESSAGE_FORWARD(AtomAutofillFrameHostMsg_HidePopup,
+      relay->window.get(), NativeWindow::HideAutofillPopup)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
@@ -1625,25 +1625,6 @@ void WebContents::OnCursorChange(const content::WebCursor& cursor) {
   }
 }
 
-void WebContents::OnShowAutofillPopup(
-    int routing_id,
-    const gfx::RectF& bounds,
-    const std::vector<base::string16>& values,
-    const std::vector<base::string16>& labels) {
-  auto relay = reinterpret_cast<NativeWindowViews*>(
-    NativeWindow::FromWebContents(web_contents()));
-  autofill_popup_->CreateView(
-    routing_id,
-    web_contents(),
-    IsOffScreen() || (embedder_ && embedder_->IsOffScreen()),
-    relay->widget(),
-    bounds);
-  autofill_popup_->SetItems(values, labels);
-}
-void WebContents::OnHideAutofillPopup() {
-  autofill_popup_->Hide();
-}
-
 void WebContents::SetSize(const SetSizeParams& params) {
   if (guest_delegate_)
     guest_delegate_->SetSize(params);
@@ -1657,6 +1638,10 @@ bool WebContents::IsOffScreen() const {
   return type_ == OFF_SCREEN;
 }
 
+bool WebContents::IsOffScreenOrEmbedderOffscreen() const {
+  return IsOffScreen() || (embedder_ && embedder_->IsOffScreen());
+}
+
 void WebContents::OnPaint(const gfx::Rect& dirty_rect, const SkBitmap& bitmap) {
   Emit("paint", dirty_rect, gfx::Image::CreateFrom1xBitmap(bitmap));
 }

atom/browser/api/atom_api_web_contents.h

@@ -181,6 +181,7 @@ class WebContents : public mate::TrackableObject<WebContents>,
 
   // Methods for offscreen rendering
   bool IsOffScreen() const;
+  bool IsOffScreenOrEmbedderOffscreen() const;
   void OnPaint(const gfx::Rect& dirty_rect, const SkBitmap& bitmap);
   void StartPainting();
   void StopPainting();
@@ -308,9 +309,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
   // content::WebContentsObserver:
   void BeforeUnloadFired(const base::TimeTicks& proceed_time) override;
   void RenderViewCreated(content::RenderViewHost*) override;
-  void RenderFrameCreated(content::RenderFrameHost*) override;
-  void RenderFrameHostChanged(content::RenderFrameHost*,
-                              content::RenderFrameHost*) override;
   void RenderViewDeleted(content::RenderViewHost*) override;
   void RenderProcessGone(base::TerminationStatus status) override;
   void DocumentLoadedInFrame(
@@ -333,6 +331,8 @@ class WebContents : public mate::TrackableObject<WebContents>,
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
   bool OnMessageReceived(const IPC::Message& message) override;
+  bool OnMessageReceived(const IPC::Message& message,
+                         content::RenderFrameHost* frame_host) override;
   void WebContentsDestroyed() override;
   void NavigationEntryCommitted(
       const content::LoadCommittedDetails& load_details) override;
@@ -378,12 +378,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
   // Called when we receive a CursorChange message from chromium.
   void OnCursorChange(const content::WebCursor& cursor);
 
-  void OnShowAutofillPopup(int routing_id,
-                           const gfx::RectF& bounds,
-                           const std::vector<base::string16>& values,
-                           const std::vector<base::string16>& labels);
-  void OnHideAutofillPopup();
-
   // Called when received a message from renderer.
   void OnRendererMessage(const base::string16& channel,
                          const base::ListValue& args);
@@ -407,7 +401,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
 
   std::unique_ptr<AtomJavaScriptDialogManager> dialog_manager_;
   std::unique_ptr<WebViewGuestDelegate> guest_delegate_;
-  AutofillPopup* autofill_popup_;
 
   // The host webcontents that may contain this webcontents.
   WebContents* embedder_;

atom/browser/native_window.h

@@ -18,10 +18,12 @@
 #include "base/observer_list.h"
 #include "base/supports_user_data.h"
 #include "content/public/browser/readback_types.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 #include "extensions/browser/app_window/size_constraints.h"
 #include "native_mate/persistent_dictionary.h"
+#include "ui/gfx/geometry/rect_f.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
 
@@ -215,6 +217,12 @@ class NativeWindow : public base::SupportsUserData,
   virtual void HandleKeyboardEvent(
       content::WebContents*,
       const content::NativeWebKeyboardEvent& event) {}
+  virtual void ShowAutofillPopup(
+    content::RenderFrameHost* frame_host,
+    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

@@ -7,6 +7,7 @@
 #include <string>
 #include <vector>
 
+#include "atom/browser/api/atom_api_web_contents.h"
 #include "atom/browser/native_browser_view_views.h"
 #include "atom/browser/ui/views/menu_bar.h"
 #include "atom/browser/window_list.h"
@@ -319,6 +320,8 @@ NativeWindowViews::NativeWindowViews(
   window_->CenterWindow(size);
   Layout();
 
+  autofill_popup_.reset(new AutofillPopup(GetNativeView()));
+
 #if defined(OS_WIN)
   // Save initial window state.
   if (fullscreen)
@@ -1269,6 +1272,26 @@ void NativeWindowViews::HandleKeyboardEvent(
   }
 }
 
+void NativeWindowViews::ShowAutofillPopup(
+    content::RenderFrameHost* frame_host,
+    const gfx::RectF& bounds,
+    const std::vector<base::string16>& values,
+    const std::vector<base::string16>& labels) {
+  auto wc = atom::api::WebContents::FromWrappedClass(
+    v8::Isolate::GetCurrent(), web_contents());
+  autofill_popup_->CreateView(
+    frame_host,
+    wc->IsOffScreenOrEmbedderOffscreen(),
+    widget(),
+    bounds);
+  autofill_popup_->SetItems(values, labels);
+}
+
+void NativeWindowViews::HideAutofillPopup(
+    content::RenderFrameHost* frame_host) {
+  autofill_popup_->Hide();
+}
+
 void NativeWindowViews::Layout() {
   const auto size = GetContentsBounds().size();
   const auto menu_bar_bounds =

atom/browser/native_window_views.h

@@ -11,6 +11,7 @@
 #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"
 
@@ -176,6 +177,12 @@ class NativeWindowViews : public NativeWindow,
   void HandleKeyboardEvent(
       content::WebContents*,
       const content::NativeWebKeyboardEvent& event) override;
+  void ShowAutofillPopup(
+    content::RenderFrameHost* frame_host,
+    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;
@@ -194,6 +201,8 @@ class NativeWindowViews : public NativeWindow,
 
   NativeBrowserView* browser_view_;
 
+  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

@@ -114,12 +114,11 @@ AutofillPopup::~AutofillPopup() {
 }
 
 void AutofillPopup::CreateView(
-    int routing_id,
+    content::RenderFrameHost* frame_host,
-    content::WebContents* web_contents,
     bool offscreen,
     views::Widget* parent_widget,
     const gfx::RectF& r) {
-  web_contents_ = web_contents;
+  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());
@@ -135,12 +134,10 @@ void AutofillPopup::CreateView(
 
   if (offscreen) {
     auto* osr_rwhv = static_cast<OffScreenRenderWidgetHostView*>(
-        web_contents->GetRenderWidgetHostView());
+        frame_host_->GetView());
     view_->view_proxy_.reset(new OffscreenViewProxy(view_));
     osr_rwhv->AddViewProxy(view_->view_proxy_.get());
   }
-
-  frame_routing_id_ = routing_id;
 }
 
 void AutofillPopup::Hide() {
@@ -161,8 +158,8 @@ void AutofillPopup::SetItems(const std::vector<base::string16>& values,
 }
 
 void AutofillPopup::AcceptSuggestion(int index) {
-  web_contents_->Send(new AtomAutofillViewMsg_AcceptSuggestion(
+  frame_host_->Send(new AtomAutofillFrameMsg_AcceptSuggestion(
-    frame_routing_id_, GetValueAt(index)));
+    frame_host_->GetRoutingID(), GetValueAt(index)));
 }
 
 void AutofillPopup::UpdatePopupBounds() {

atom/browser/ui/autofill_popup.h

@@ -8,7 +8,7 @@
 #include <vector>
 
 #include "atom/browser/ui/views/autofill_popup_view.h"
-#include "content/public/browser/web_contents.h"
+#include "content/public/browser/render_frame_host.h"
 #include "ui/gfx/font_list.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/widget/widget.h"
@@ -22,7 +22,7 @@ class AutofillPopup {
   explicit AutofillPopup(gfx::NativeView);
   ~AutofillPopup();
 
-  void CreateView(int routing_id, content::WebContents* web_contents,
+  void CreateView(content::RenderFrameHost* render_frame,
     bool offscreen, views::Widget* widget, const gfx::RectF& bounds);
   void Hide();
 
@@ -69,10 +69,9 @@ class AutofillPopup {
 
   // For sending the accepted suggestion to the render frame that
   // asked to open the popup
-  int frame_routing_id_;
+  content::RenderFrameHost* frame_host_;
-  content::WebContents* web_contents_;
 
-  // The popup view
+  // The popup view. The lifetime is managed by the owning Widget
   AutofillPopupView* view_;
 
   DISALLOW_COPY_AND_ASSIGN(AutofillPopup);

atom/browser/ui/views/autofill_popup_view.cc

@@ -31,8 +31,9 @@ AutofillPopupView::AutofillPopupView(
 
 AutofillPopupView::~AutofillPopupView() {
   if (popup_) {
-    auto host = popup_->web_contents_->GetRenderViewHost()->GetWidget();
+    auto host = popup_->frame_host_->GetRenderViewHost()->GetWidget();
     host->RemoveKeyPressEventCallback(keypress_callback_);
+    popup_->view_ = nullptr;
     popup_ = nullptr;
   }
 
@@ -92,23 +93,24 @@ void AutofillPopupView::Show() {
 
   keypress_callback_ = base::Bind(&AutofillPopupView::HandleKeyPressEvent,
     base::Unretained(this));
-  auto host = popup_->web_contents_->GetRenderViewHost()->GetWidget();
+  auto host = popup_->frame_host_->GetRenderViewHost()->GetWidget();
   host->AddKeyPressEventCallback(keypress_callback_);
+
+  NotifyAccessibilityEvent(ui::AX_EVENT_MENU_START, true);
 }
 
 void AutofillPopupView::Hide() {
   if (popup_) {
-    auto host = popup_->web_contents_->GetRenderViewHost()->GetWidget();
+    auto host = popup_->frame_host_->GetRenderViewHost()->GetWidget();
     host->RemoveKeyPressEventCallback(keypress_callback_);
-    popup_ = NULL;
+    popup_ = nullptr;
   }
 
   RemoveObserver();
+  NotifyAccessibilityEvent(ui::AX_EVENT_MENU_END, true);
 
   if (GetWidget()) {
     GetWidget()->Close();
-  } else {
-    delete this;
   }
 }
 

atom/common/api/api_messages.h

@@ -38,20 +38,16 @@ IPC_MESSAGE_ROUTED3(AtomViewMsg_Message,
 
 IPC_MESSAGE_ROUTED0(AtomViewMsg_Offscreen)
 
-IPC_MESSAGE_ROUTED4(AtomAutofillViewMsg_ShowPopup,
+IPC_MESSAGE_ROUTED3(AtomAutofillFrameHostMsg_ShowPopup,
-                    int /* routing_id */,
                     gfx::RectF /* bounds */,
                     std::vector<base::string16> /* values */,
                     std::vector<base::string16> /* labels */)
 
-IPC_MESSAGE_ROUTED0(AtomAutofillViewMsg_HidePopup)
+IPC_MESSAGE_ROUTED0(AtomAutofillFrameHostMsg_HidePopup)
 
-IPC_MESSAGE_ROUTED1(AtomAutofillViewMsg_AcceptSuggestion,
+IPC_MESSAGE_ROUTED1(AtomAutofillFrameMsg_AcceptSuggestion,
                     base::string16 /* suggestion */)
 
-IPC_MESSAGE_ROUTED1(AtomAutofillViewHostMsg_RoutingId,
-                    int /* id */)
-
 // Sent by the renderer when the draggable regions are updated.
 IPC_MESSAGE_ROUTED1(AtomViewHostMsg_UpdateDraggableRegions,
                     std::vector<atom::DraggableRegion> /* regions */)

atom/renderer/atom_autofill_agent.cc

@@ -52,16 +52,11 @@ void TrimStringVectorForIPC(std::vector<base::string16>* strings) {
 AutofillAgent::AutofillAgent(
     content::RenderFrame* frame)
   : content::RenderFrameObserver(frame),
-    render_frame_(frame),
     helper_(new Helper(this)),
     focused_node_was_last_clicked_(false),
     was_focused_before_now_(false),
     weak_ptr_factory_(this) {
-  render_frame_->GetWebFrame()->setAutofillClient(this);
+  render_frame()->GetWebFrame()->setAutofillClient(this);
-}
-
-AutofillAgent::~AutofillAgent() {
-  delete helper_;
 }
 
 void AutofillAgent::OnDestruct() {
@@ -169,7 +164,7 @@ void AutofillAgent::ShowSuggestions(
 }
 
 AutofillAgent::Helper::Helper(AutofillAgent* agent)
-  : content::RenderViewObserver(agent->render_frame_->GetRenderView()),
+  : content::RenderViewObserver(agent->render_frame()->GetRenderView()),
     agent_(agent) {
 }
 
@@ -184,9 +179,7 @@ void AutofillAgent::Helper::FocusChangeComplete() {
 bool AutofillAgent::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(AutofillAgent, message)
-    IPC_MESSAGE_HANDLER(AtomAutofillViewHostMsg_RoutingId,
+    IPC_MESSAGE_HANDLER(AtomAutofillFrameMsg_AcceptSuggestion,
-      OnWebContentsRoutingIdReceived)
-    IPC_MESSAGE_HANDLER(AtomAutofillViewMsg_AcceptSuggestion,
       OnAcceptSuggestion)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
@@ -194,16 +187,12 @@ bool AutofillAgent::OnMessageReceived(const IPC::Message& message) {
   return handled;
 }
 
-void AutofillAgent::OnWebContentsRoutingIdReceived(int id) {
-  web_contents_routing_id_ = id;
-}
-
 bool AutofillAgent::IsUserGesture() const {
   return blink::WebUserGestureIndicator::isProcessingUserGesture();
 }
 
 void AutofillAgent::HidePopup() {
-  Send(new AtomAutofillViewMsg_HidePopup(web_contents_routing_id_));
+  Send(new AtomAutofillFrameHostMsg_HidePopup(render_frame()->GetRoutingID()));
 }
 
 void AutofillAgent::ShowPopup(
@@ -211,13 +200,13 @@ void AutofillAgent::ShowPopup(
     const std::vector<base::string16>& values,
     const std::vector<base::string16>& labels) {
   gfx::RectF bounds =
-    render_frame_->GetRenderView()->ElementBoundsInWindow(element);
+    render_frame()->GetRenderView()->ElementBoundsInWindow(element);
-  Send(new AtomAutofillViewMsg_ShowPopup(
+  Send(new AtomAutofillFrameHostMsg_ShowPopup(
-    web_contents_routing_id_, routing_id(), bounds, values, labels));
+    render_frame()->GetRoutingID(), bounds, values, labels));
 }
 
 void AutofillAgent::OnAcceptSuggestion(base::string16 suggestion) {
-  auto element = render_frame_->GetWebFrame()->document().focusedElement();
+  auto element = render_frame()->GetWebFrame()->document().focusedElement();
   if (element.isFormControlElement()) {
     toWebInputElement(&element)->setSuggestedValue(
       blink::WebString::fromUTF16(suggestion));
@@ -225,7 +214,7 @@ void AutofillAgent::OnAcceptSuggestion(base::string16 suggestion) {
 }
 
 void AutofillAgent::DoFocusChangeComplete() {
-  auto element = render_frame_->GetWebFrame()->document().focusedElement();
+  auto element = render_frame()->GetWebFrame()->document().focusedElement();
   if (element.isNull() || !element.isFormControlElement())
     return;
 

atom/renderer/atom_autofill_agent.h

@@ -21,7 +21,6 @@ class AutofillAgent : public content::RenderFrameObserver,
                       public blink::WebAutofillClient {
  public:
   explicit AutofillAgent(content::RenderFrame* frame);
-  ~AutofillAgent();
 
   // content::RenderFrameObserver:
   void OnDestruct() override;
@@ -51,7 +50,6 @@ class AutofillAgent : public content::RenderFrameObserver,
   };
 
   bool OnMessageReceived(const IPC::Message& message) override;
-  void OnWebContentsRoutingIdReceived(int routing_id);
 
   // blink::WebAutofillClient:
   void textFieldDidEndEditing(const blink::WebInputElement&) override;
@@ -73,10 +71,7 @@ class AutofillAgent : public content::RenderFrameObserver,
 
   void DoFocusChangeComplete();
 
-  content::RenderFrame* render_frame_;
+  std::unique_ptr<Helper> helper_;
-  int web_contents_routing_id_;
-
-  Helper* helper_;
 
   // True when the last click was on the focused node.
   bool focused_node_was_last_clicked_;