Bug 1384027 - part2: Move PuppetWidget::NotifyIMEInternal() implementation to PuppetWidget::NotifyIME() which is a method of TextEventDispatcherListener, not nsIWidget r=m_kato

nsIWidget::NotifyIME() should call only TextEventDispatcher::NotifyIME() if it's necessary.  Then, TextEventDispatcher::NotifyIME() calls TextEventDispatcherListener::NotifyIME() if it's necessary.  E.g., requests to IME are necessary only for TextInputProcessor or native IME handler because the composition is only owned by one of them.  However, notifications are necessary for both of them since focused editor contents and its focus state are shared.

So, it doesn't need to call nsBaseWidget::NotifyIMEInternal() if all NotifyIMEInternal() implementations are moved to proper TextEventDispatcherListener::NotifyIME().  Currently, nsBaseWidget::NotifyIMEInternal() is implemented only by PuppetWidget.  It sends notifications and requests to the parent process for native IME.  Therefore, we can move NotifyIMEInternal() implementation to TextEventDispatcherListener::NotifyIME() which is implemented by PuppetWidget.

This patch moves PuppetWidget::NotifyIMEInternal() implementation to PuppetWidget::NotifyIME() of TextEventDispatcherListener class, not of nsIWidget and removes NotifyIMEInternal() completely.

With this change, handling order is changed.  Old behavior is, TextEventDispatcher::NotifyIME() calls TextEventDispatcherListener::NotifyIME() before handling NOTIFY_IME_OF_FOCUS and then, nsBaseWidget::NotifyIME() sends the notification to the parent process.  However, new behavior is, the notification is sent before TextEventDispatcher::NotifyIME() handles NOTIFY_IME_OF_FOCUS.  Therefore, with new handling order, TextEventDispatcher can have IME notification requests after setting focus correctly.  Additionally, TextEventDispatcher for PuppetWidget updates the notification requests at every event dispatch via TextEventDispatcher::BeginInputTransactionInternal() by the previous patch.

So, with those patches, IMEContentObserver can refer actual IME notification requests correctly even after we'll make focus notification to async message.

MozReview-Commit-ID: JwdQ68BjTXL

widget/PuppetWidget.cpp

@@ -690,37 +690,6 @@ PuppetWidget::RequestIMEToCommitComposition(bool aCancel)
   return NS_OK;
 }
 
-nsresult
-PuppetWidget::NotifyIMEInternal(const IMENotification& aIMENotification)
-{
-  if (mNativeTextEventDispatcherListener) {
-    // Use mNativeTextEventDispatcherListener for IME notifications.
-    return NS_ERROR_NOT_IMPLEMENTED;
-  }
-
-  switch (aIMENotification.mMessage) {
-    case REQUEST_TO_COMMIT_COMPOSITION:
-      return RequestIMEToCommitComposition(false);
-    case REQUEST_TO_CANCEL_COMPOSITION:
-      return RequestIMEToCommitComposition(true);
-    case NOTIFY_IME_OF_FOCUS:
-    case NOTIFY_IME_OF_BLUR:
-      return NotifyIMEOfFocusChange(aIMENotification);
-    case NOTIFY_IME_OF_SELECTION_CHANGE:
-      return NotifyIMEOfSelectionChange(aIMENotification);
-    case NOTIFY_IME_OF_TEXT_CHANGE:
-      return NotifyIMEOfTextChange(aIMENotification);
-    case NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED:
-      return NotifyIMEOfCompositionUpdate(aIMENotification);
-    case NOTIFY_IME_OF_MOUSE_BUTTON_EVENT:
-      return NotifyIMEOfMouseButtonEvent(aIMENotification);
-    case NOTIFY_IME_OF_POSITION_CHANGE:
-      return NotifyIMEOfPositionChange(aIMENotification);
-    default:
-      return NS_ERROR_NOT_IMPLEMENTED;
-  }
-}
-
 nsresult
 PuppetWidget::StartPluginIME(const mozilla::WidgetKeyboardEvent& aKeyboardEvent,
                              int32_t aPanelX, int32_t aPanelY,
@@ -1492,9 +1461,40 @@ PuppetWidget::OnWindowedPluginKeyEvent(const NativeEventData& aKeyEventData,
 
 NS_IMETHODIMP
 PuppetWidget::NotifyIME(TextEventDispatcher* aTextEventDispatcher,
-                        const IMENotification& aNotification)
+                        const IMENotification& aIMENotification)
 {
   MOZ_ASSERT(aTextEventDispatcher == mTextEventDispatcher);
+
+  // If there is different text event dispatcher listener for handling
+  // text event dispatcher, that means that native keyboard events and
+  // IME events are handled in this process.  Therefore, we don't need
+  // to send any requests and notifications to the parent process.
+  if (mNativeTextEventDispatcherListener) {
+    return NS_ERROR_NOT_IMPLEMENTED;
+  }
+
+  switch (aIMENotification.mMessage) {
+    case REQUEST_TO_COMMIT_COMPOSITION:
+      return RequestIMEToCommitComposition(false);
+    case REQUEST_TO_CANCEL_COMPOSITION:
+      return RequestIMEToCommitComposition(true);
+    case NOTIFY_IME_OF_FOCUS:
+    case NOTIFY_IME_OF_BLUR:
+      return NotifyIMEOfFocusChange(aIMENotification);
+    case NOTIFY_IME_OF_SELECTION_CHANGE:
+      return NotifyIMEOfSelectionChange(aIMENotification);
+    case NOTIFY_IME_OF_TEXT_CHANGE:
+      return NotifyIMEOfTextChange(aIMENotification);
+    case NOTIFY_IME_OF_COMPOSITION_EVENT_HANDLED:
+      return NotifyIMEOfCompositionUpdate(aIMENotification);
+    case NOTIFY_IME_OF_MOUSE_BUTTON_EVENT:
+      return NotifyIMEOfMouseButtonEvent(aIMENotification);
+    case NOTIFY_IME_OF_POSITION_CHANGE:
+      return NotifyIMEOfPositionChange(aIMENotification);
+    default:
+      return NS_ERROR_NOT_IMPLEMENTED;
+  }
+
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 

widget/PuppetWidget.h

@@ -315,10 +315,6 @@ public:
                       uint32_t aIndexOfKeypress,
                       void* aData) override;
 
-protected:
-  virtual nsresult NotifyIMEInternal(
-                     const IMENotification& aIMENotification) override;
-
 private:
   nsresult Paint();
 

widget/nsBaseWidget.cpp

@@ -1797,31 +1797,31 @@ nsBaseWidget::NotifyIME(const IMENotification& aIMENotification)
   switch (aIMENotification.mMessage) {
     case REQUEST_TO_COMMIT_COMPOSITION:
     case REQUEST_TO_CANCEL_COMPOSITION:
-      // Currently, if native IME handler doesn't use TextEventDispatcher,
+      // We should send request to IME only when there is a TextEventDispatcher
-      // the request may be notified to mTextEventDispatcher or native IME
+      // instance (this means that this widget has dispatched at least one
-      // directly.  Therefore, if mTextEventDispatcher has a composition,
+      // composition event or keyboard event) and the it has composition.
-      // the request should be handled by the mTextEventDispatcher.
+      // Otherwise, there is nothing to do.
+      // Note that if current input transaction is for native input events,
+      // TextEventDispatcher::NotifyIME() will call
+      // TextEventDispatcherListener::NotifyIME().
       if (mTextEventDispatcher && mTextEventDispatcher->IsComposing()) {
         return mTextEventDispatcher->NotifyIME(aIMENotification);
       }
-      // Otherwise, it should be handled by native IME.
+      return NS_OK;
-      return NotifyIMEInternal(aIMENotification);
     default: {
       if (aIMENotification.mMessage == NOTIFY_IME_OF_FOCUS) {
         mIMEHasFocus = true;
       }
       EnsureTextEventDispatcher();
-      // If the platform specific widget uses TextEventDispatcher for handling
+      // TextEventDispatcher::NotifyIME() will always call
-      // native IME and keyboard events, IME event handler should be notified
+      // TextEventDispatcherListener::NotifyIME().  I.e., even if current
-      // of the notification via TextEventDispatcher.  Otherwise, on the other
+      // input transaction is for synthesized events for automated tests,
-      // platforms which have not used TextEventDispatcher yet, IME event
+      // notifications will be sent to native IME.
-      // handler should be notified by the old path (NotifyIMEInternal).
       nsresult rv = mTextEventDispatcher->NotifyIME(aIMENotification);
-      nsresult rv2 = NotifyIMEInternal(aIMENotification);
       if (aIMENotification.mMessage == NOTIFY_IME_OF_BLUR) {
         mIMEHasFocus = false;
       }
-      return rv2 == NS_ERROR_NOT_IMPLEMENTED ? rv : rv2;
+      return rv;
     }
   }
 }

widget/nsBaseWidget.h

@@ -521,9 +521,6 @@ protected:
     return NS_ERROR_UNEXPECTED;
   }
 
-  virtual nsresult NotifyIMEInternal(const IMENotification& aIMENotification)
-  { return NS_ERROR_NOT_IMPLEMENTED; }
-
   /**
    * GetPseudoIMEContext() returns pseudo IME context when TextEventDispatcher
    * has non-native input transaction.  Otherwise, returns nullptr.