MacViews: Replace NativeWidget::GetEventHandler() with RepostNativeEvent(..)

ui/views/controls/webview/web_dialog_view.cc

 // Copyright (c) 2012 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.
 
 #include "ui/views/controls/webview/web_dialog_view.h"
 
 #include <vector>
 
 #include "base/strings/utf_string_conversions.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/events/event.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/views/controls/webview/webview.h"
 #include "ui/views/layout/fill_layout.h"
-#include "ui/views/widget/native_widget_aura.h"
+#include "ui/views/widget/native_widget.h"
 #include "ui/views/widget/root_view.h"
 #include "ui/views/widget/widget.h"
 #include "ui/web_dialogs/web_dialog_delegate.h"
 #include "ui/web_dialogs/web_dialog_ui.h"
 
 using content::NativeWebKeyboardEvent;
 using content::WebContents;
 using content::WebUIMessageHandler;
 using ui::WebDialogDelegate;
 using ui::WebDialogUI;
@@ skipping 241 matching lines @@
   GetWidget()->SetBounds(pos);
 }
 
 // A simplified version of BrowserView::HandleKeyboardEvent().
 // We don't handle global keyboard shortcuts here, but that's fine since
 // they're all browser-specific. (This may change in the future.)
 void WebDialogView::HandleKeyboardEvent(content::WebContents* source,
                                         const NativeWebKeyboardEvent& event) {
   if (!event.os_event)
     return;
-  ui::KeyEvent aura_event(event.os_event->native_event(), false);

[tapted, 2014/05/28 12:36:21]

note that the native event is already copied in
native_web_keyboard_event_aura.cc, so removing this extra copy step should be
fine

[sadrul, 2014/05/29 23:09:27]

Can you confirm that event.os_event->is_char() is always set to false for aura
here? Because if it's not, then the behaviour is going to be different. A
[D]CHECK would be a good idea.

[tapted, 2014/05/29 23:36:19]

Oooh - that's a good point. I'm not sure it's true in general here, and the
change might be an improvement..

is_char isn't too familiar to me, so I'm doing some research (still poking
around). However, my initial feeling is that the `false` that was here isn't
deliberate, and possibly a subtle bug.

This code was added so that `ESC` in a WebDialogView could close it, via
WebDialogView::AcceleratorPressed. AcceleratorFilter::OnKeyEvent will filter out
events where is_char is true. So the old code has the behaviour of turning
*translated* character events into accelerators, which doesn't seem right.

I've checked for regressions (took me a while to find a dialog that still used
this!), and ESC still works to close the dialog.

(note: an example dialog is the 'Print using cloud print' dialog -- a link to
open it appears in the Ctrl+P dialog but only *after* selecting a cloud printer
as the destination.)

[tapted, 2014/05/30 01:59:44]

Also note the non-aura version on Windows would just DefWindowProc without
changes to the event (see e.g. http://crrev.com/118343 ), which also suggests to
me that the unconditional `is_char = false` was unintentional.

If you prefer I can check it here. However, it would mean bringing back an #if
defined(USE_AURA) which I'm also hoping to avoid :)

-  ui::EventHandler* event_handler =
-      GetWidget()->native_widget()->GetEventHandler();
 
-  DCHECK(event_handler);
-  if (event_handler)
-    event_handler->OnKeyEvent(&aura_event);
-
+  GetWidget()->native_widget()->RepostNativeEvent(event.os_event);
 }
 
 void WebDialogView::CloseContents(WebContents* source) {
   close_contents_called_ = true;
   bool close_dialog = false;
   OnCloseContents(source, &close_dialog);
   if (close_dialog)
     OnDialogClosed(closed_via_webui_ ? dialog_close_retval_ : std::string());
 }
 
@@ skipping 47 matching lines @@
   web_contents->SetDelegate(this);
 
   // Set the delegate. This must be done before loading the page. See
   // the comment above WebDialogUI in its header file for why.
   WebDialogUI::SetDelegate(web_contents, this);
 
   web_view_->LoadInitialURL(GetDialogContentURL());
 }
 
 }  // namespace views