Fix autofill popup controller key press callback registration

chrome/browser/ui/autofill/popup_controller_common.cc

Index: chrome/browser/ui/autofill/popup_controller_common.cc
diff --git a/chrome/browser/ui/autofill/popup_controller_common.cc b/chrome/browser/ui/autofill/popup_controller_common.cc
index 2c97b8b7c222c6fb58f61eebf43f28da21da1208..964f25bad9575cbbfa34bc384fada0837e63dfc5 100644
--- a/chrome/browser/ui/autofill/popup_controller_common.cc
+++ b/chrome/browser/ui/autofill/popup_controller_common.cc
@@ -4,11 +4,27 @@
 
 #include "chrome/browser/ui/autofill/popup_controller_common.h"
 
-#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 
 namespace autofill {
 
+namespace {
+
+content::RenderWidgetHost* KeyPressHandlingTarget(

[Mathieu, 2017/03/26 02:03:51]

since it's in the anonymous namespace, a short function comment would be good.

[vabr (Chromium), 2017/03/26 03:25:40]

Done.

+    content::WebContents* web_contents) {
+  content::RenderFrameHost* frame = web_contents->GetFocusedFrame();
+  if (!frame)
+    frame = web_contents->GetMainFrame();
+  content::RenderWidgetHostView* view = frame->GetView();
+  if (!view)
+    return nullptr;

[Mathieu, 2017/03/26 02:03:51]

this is a sure crash on line 50, no?

[vabr (Chromium), 2017/03/26 03:25:40]

For some reason I only saw the view being null during RemoveKeyPressCallback. My
guess is that during some shutdown sequence this might happen.

During the registering phase, there is no good way to handle this gracefully --
if there is no view and hence RenderWidgetHost, then autofill cannot listen to
keys pressed for the suggestions pop-up. That should not happen, and if it does,
we might as well have a crash to highlight it.

+  return view->GetRenderWidgetHost();
+}
+
+}  // namespace
+
 PopupControllerCommon::PopupControllerCommon(
     const gfx::RectF& element_bounds,
     base::i18n::TextDirection text_direction,
@@ -30,18 +46,17 @@ void PopupControllerCommon::SetKeyPressCallback(
 
 void PopupControllerCommon::RegisterKeyPressCallback() {
   if (web_contents_ && !key_press_event_target_) {
-    key_press_event_target_ = web_contents_->GetRenderViewHost();
-    key_press_event_target_->GetWidget()->AddKeyPressEventCallback(
+    key_press_event_target_ = KeyPressHandlingTarget(web_contents_);
+    key_press_event_target_->AddKeyPressEventCallback(
         key_press_event_callback_);
   }
 }
 
 void PopupControllerCommon::RemoveKeyPressCallback() {
   if (web_contents_ && (!web_contents_->IsBeingDestroyed()) &&
-      key_press_event_target_ == web_contents_->GetRenderViewHost()) {
-    web_contents_->GetRenderViewHost()
-        ->GetWidget()
-        ->RemoveKeyPressEventCallback(key_press_event_callback_);
+      key_press_event_target_ == KeyPressHandlingTarget(web_contents_)) {
+    key_press_event_target_->RemoveKeyPressEventCallback(
+        key_press_event_callback_);
   }
   key_press_event_target_ = NULL;
 }