Bug 1300003 part.6 NativeKey shouldn't try to dispatch plugin events for removed char messages when mWidget won't dispatch plugin events r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 15 Sep 2016 00:16:18 +0900
changeset 413992 71f477b9e3b20af4a57170667b75a686453a039e
parent 413991 a77662f5a55b71dba06f333e85a72cce120637c4
child 531352 70b3834bb5cf41eeb9f430f9bc4811582f787cc2
push id29569
push usermasayuki@d-toybox.com
push dateThu, 15 Sep 2016 12:25:58 +0000
reviewersm_kato
bugs1300003, 1297013
milestone51.0a1
Bug 1300003 part.6 NativeKey shouldn't try to dispatch plugin events for removed char messages when mWidget won't dispatch plugin events r?m_kato Currently, NativeKey::DispatchPluginEventsAndDiscardsCharMessages() calls nsWindowBase::DispatchPluginEvent() and nsWindowBase::DispatchPluginEvent() does nothing when windowless plugin doesn't have focus. However, this is unclear when other developers read the code of DispatchPluginEventsAndDiscardsCharMessages() and it causes unnecessary virtual calls. So, it should try to dispatch plugin events only when a windowless plugin has focus. For making the check safer, nsWindowBase should expose the method to check it as ShouldDispatchPluginEvent(). Unfortunately, we cannot mark this method as const due to PluginHasFocus() needs to be not const method, though. Then, for loops in DispatchPluginEventsAndDiscardsCharMessages() should check it at each try. This change helps to log the behavior (working on bug 1297013) without noise. Additionally, this patch renames the method to MaybeDispatchPluginEventsForRemovedCharMessages() because it doesn't remove any char messages anymore. MozReview-Commit-ID: F14Lcx47M6U
widget/windows/KeyboardLayout.cpp
widget/windows/KeyboardLayout.h
widget/windows/nsWindowBase.cpp
widget/windows/nsWindowBase.h
--- a/widget/windows/KeyboardLayout.cpp
+++ b/widget/windows/KeyboardLayout.cpp
@@ -1838,24 +1838,24 @@ NativeKey::HandleKeyDownMessage(bool* aE
   RedirectedKeyDownMessageManager::Forget();
 
   // If the key was processed by IME, we shouldn't dispatch keypress event.
   if (mOriginalVirtualKeyCode == VK_PROCESSKEY) {
     return defaultPrevented;
   }
 
   if (defaultPrevented) {
-    DispatchPluginEventsAndDiscardsCharMessages();
+    MaybeDispatchPluginEventsForRemovedCharMessages();
     return true;
   }
 
   // If we won't be getting a WM_CHAR, WM_SYSCHAR or WM_DEADCHAR, synthesize a
   // keypress for almost all keys
   if (NeedsToHandleWithoutFollowingCharMessages()) {
-    return (DispatchPluginEventsAndDiscardsCharMessages() ||
+    return (MaybeDispatchPluginEventsForRemovedCharMessages() ||
             DispatchKeyPressEventsWithoutCharMessage());
   }
 
   if (!mFollowingCharMsgs.IsEmpty()) {
     bool consumed = false;
     for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
       consumed =
         DispatchKeyPressEventForFollowingCharMessage(mFollowingCharMsgs[i]) ||
@@ -2377,35 +2377,38 @@ NativeKey::GetFollowingCharMessage(MSG& 
                        nextKeyMsg.message, nextKeyMsg.wParam,
                        nextKeyMsg.lParam);
   CrashReporter::AppendAppNotesToCrashReport(info);
 #endif // #ifdef MOZ_CRASHREPORTER
   MOZ_CRASH("We lost the following char message");
   return false;
 }
 
-// TODO: Rename this method later.
 bool
-NativeKey::DispatchPluginEventsAndDiscardsCharMessages() const
+NativeKey::MaybeDispatchPluginEventsForRemovedCharMessages() const
 {
   MOZ_ASSERT(IsKeyDownMessage());
   MOZ_ASSERT(!IsKeyMessageOnPlugin());
 
-  for (size_t i = 0; i < mFollowingCharMsgs.Length(); ++i) {
+  for (size_t i = 0;
+       i < mFollowingCharMsgs.Length() && mWidget->ShouldDispatchPluginEvent();
+       ++i) {
     MOZ_RELEASE_ASSERT(!mWidget->Destroyed(),
       "NativeKey tries to dispatch a plugin event on destroyed widget");
     mWidget->DispatchPluginEvent(mFollowingCharMsgs[i]);
     if (mWidget->Destroyed() || IsFocusedWindowChanged()) {
       return true;
     }
   }
 
   // Dispatch odd char messages which are caused by ATOK or WXG (both of them
   // are Japanese IME) and removed by RemoveFollowingOddCharMessages().
-  for (size_t i = 0; i < mRemovedOddCharMsgs.Length(); ++i) {
+  for (size_t i = 0;
+       i < mRemovedOddCharMsgs.Length() && mWidget->ShouldDispatchPluginEvent();
+       ++i) {
     MOZ_RELEASE_ASSERT(!mWidget->Destroyed(),
       "NativeKey tries to dispatch a plugin event on destroyed widget");
     mWidget->DispatchPluginEvent(mRemovedOddCharMsgs[i]);
     if (mWidget->Destroyed() || IsFocusedWindowChanged()) {
       return true;
     }
   }
 
--- a/widget/windows/KeyboardLayout.h
+++ b/widget/windows/KeyboardLayout.h
@@ -499,22 +499,22 @@ private:
   /**
    * DispatchKeyPressEventsWithoutCharMessage() dispatches keypress event(s)
    * without char messages.  So, this should be used only when there are no
    * following char messages.
    */
   bool DispatchKeyPressEventsWithoutCharMessage() const;
 
   /**
-   * Remove all following WM_CHAR, WM_SYSCHAR and WM_DEADCHAR messages for the
-   * WM_KEYDOWN or WM_SYSKEYDOWN message.  Additionally, dispatches plugin
-   * events if it's necessary.
-   * Returns true if the widget is destroyed.  Otherwise, false.
+   * MaybeDispatchPluginEventsForRemovedCharMessages() dispatches plugin events
+   * for removed char messages when a windowless plugin has focus.
+   * Returns true if the widget is destroyed or blurred during dispatching a
+   * plugin event.
    */
-  bool DispatchPluginEventsAndDiscardsCharMessages() const;
+  bool MaybeDispatchPluginEventsForRemovedCharMessages() const;
 
   /**
    * DispatchKeyPressEventForFollowingCharMessage() dispatches keypress event
    * for following WM_*CHAR message which is removed and set to aCharMsg.
    * Returns true if the event is consumed.  Otherwise, false.
    */
   bool DispatchKeyPressEventForFollowingCharMessage(const MSG& aCharMsg) const;
 
--- a/widget/windows/nsWindowBase.cpp
+++ b/widget/windows/nsWindowBase.cpp
@@ -16,31 +16,37 @@ using namespace mozilla::widget;
 
 static const wchar_t kUser32LibName[] =  L"user32.dll";
 bool nsWindowBase::sTouchInjectInitialized = false;
 InjectTouchInputPtr nsWindowBase::sInjectTouchFuncPtr;
 
 bool
 nsWindowBase::DispatchPluginEvent(const MSG& aMsg)
 {
-  if (!PluginHasFocus()) {
+  if (!ShouldDispatchPluginEvent()) {
     return false;
   }
   WidgetPluginEvent pluginEvent(true, ePluginInputEvent, this);
   LayoutDeviceIntPoint point(0, 0);
   InitEvent(pluginEvent, &point);
   NPEvent npEvent;
   npEvent.event = aMsg.message;
   npEvent.wParam = aMsg.wParam;
   npEvent.lParam = aMsg.lParam;
   pluginEvent.mPluginEvent.Copy(npEvent);
   pluginEvent.mRetargetToFocusedDocument = true;
   return DispatchWindowEvent(&pluginEvent);
 }
 
+bool
+nsWindowBase::ShouldDispatchPluginEvent()
+{
+  return PluginHasFocus();
+}
+
 // static
 bool
 nsWindowBase::InitTouchInjection()
 {
   if (!sTouchInjectInitialized) {
     // Initialize touch injection on the first call
     HMODULE hMod = LoadLibraryW(kUser32LibName);
     if (!hMod) {
--- a/widget/windows/nsWindowBase.h
+++ b/widget/windows/nsWindowBase.h
@@ -81,16 +81,21 @@ public:
   virtual bool DispatchContentCommandEvent(mozilla::WidgetContentCommandEvent* aEvent) = 0;
 
   /*
    * Default dispatch of a plugin event.
    */
   virtual bool DispatchPluginEvent(const MSG& aMsg);
 
   /*
+   * Returns true if this should dispatch a plugin event.
+   */
+  bool ShouldDispatchPluginEvent();
+
+  /*
    * Touch input injection apis
    */
   virtual nsresult SynthesizeNativeTouchPoint(uint32_t aPointerId,
                                               TouchPointerState aPointerState,
                                               LayoutDeviceIntPoint aPoint,
                                               double aPointerPressure,
                                               uint32_t aPointerOrientation,
                                               nsIObserver* aObserver) override;