Bug 1396725 - IMEStateManager doesn't need to manage whether menu keyboard listener is installed in different process r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 07 Sep 2017 11:46:08 +0900
changeset 661155 ed61960d106279379c912720f55179efade3007f
parent 660422 d8e238b811d3dc74515065ae8cab6c74baf0295f
child 730470 af4a9f0fec19418c331528ec90de8ad629b5b23e
push id78649
push usermasayuki@d-toybox.com
push dateFri, 08 Sep 2017 02:04:30 +0000
reviewerssmaug
bugs1396725
milestone57.0a1
Bug 1396725 - IMEStateManager doesn't need to manage whether menu keyboard listener is installed in different process r?smaug Currently, IMEStateManager::OnChangeFocusInternal() tries to sync the state whether menu keyboard listener is installed between itself and active remote process -- When menu keyboard listener is installed, it posts a message to _only_ active remote process. When menu keyboard listener is uninstalled, it posts a message to _only_ active remote process. So, it's not guaranteed that active remote process at installing and uninstalling may be different. If it's different, IMEStateManager in the old remote process believes that menu keyboard listener is still installed. This is what the cause of IME unavailable in a remote process. Current approach must be wrong. IMEStateManager should manage menu keyboard listener state only in the process which the listener is installed in. Then, when menu keyboard listener is uninstalled, IMEStateManager needs to restore the latest input context in the remote process without asking the remote process. Therefore, this patch does: * stops IMEStateManager::OnChangeFocusInternal() posting message when menu keyboard listener is installed and uninstalled. * removes the message sender and receiver from PBrowser. * cache the latest input context of active remote process in IMEStateManager::SetInputContextForChildProcess(). * make IMEStateManager::SetInputContextForChildProcess() not set input context when menu keyboard listener is installed in the process. * tries to restore latest input context in the remote process in IMEStateManager::OnChangeFocusInternal(). If there is no cached input context, it does nothing and waits next SetInputContextForChildProcess() call. * clears the cache when IMEStateManager::OnChangeFocusInternal() changes active remote process to different one or nullptr. So, this must improve performance at activating and inactivating memubar and opening and closing popup menu in the main process. MozReview-Commit-ID: EelKSPlaXdw
dom/events/IMEStateManager.cpp
dom/events/IMEStateManager.h
dom/ipc/PBrowser.ipdl
dom/ipc/TabChild.cpp
dom/ipc/TabChild.h
--- a/dom/events/IMEStateManager.cpp
+++ b/dom/events/IMEStateManager.cpp
@@ -152,16 +152,17 @@ StaticRefPtr<nsPresContext> IMEStateMana
 nsIWidget* IMEStateManager::sWidget = nullptr;
 nsIWidget* IMEStateManager::sFocusedIMEWidget = nullptr;
 StaticRefPtr<TabParent> IMEStateManager::sFocusedIMETabParent;
 nsIWidget* IMEStateManager::sActiveInputContextWidget = nullptr;
 StaticRefPtr<TabParent> IMEStateManager::sActiveTabParent;
 StaticRefPtr<IMEContentObserver> IMEStateManager::sActiveIMEContentObserver;
 TextCompositionArray* IMEStateManager::sTextCompositions = nullptr;
 InputContext::Origin IMEStateManager::sOrigin = InputContext::ORIGIN_MAIN;
+InputContext IMEStateManager::sActiveChildInputContext;
 bool IMEStateManager::sInstalledMenuKeyboardListener = false;
 bool IMEStateManager::sIsGettingNewIMEState = false;
 bool IMEStateManager::sCheckForIMEUnawareWebApps = false;
 bool IMEStateManager::sInputModeSupported = false;
 
 // static
 void
 IMEStateManager::Init()
@@ -173,16 +174,17 @@ IMEStateManager::Init()
 
   Preferences::AddBoolVarCache(
     &sInputModeSupported,
     "dom.forms.inputmode",
     false);
 
   sOrigin = XRE_IsParentProcess() ? InputContext::ORIGIN_MAIN :
                                     InputContext::ORIGIN_CONTENT;
+  ResetActiveChildInputContext();
 }
 
 // static
 void
 IMEStateManager::Shutdown()
 {
   MOZ_LOG(sISMLog, LogLevel::Info,
     ("Shutdown(), sTextCompositions=0x%p, sTextCompositions->Length()=%zu",
@@ -588,35 +590,44 @@ IMEStateManager::OnChangeFocusInternal(n
   // are handled by IME.
   IMEState newState =
     newTabParent ? IMEState(IMEState::DISABLED) :
                    GetNewIMEState(aPresContext, aContent);
   bool setIMEState = true;
 
   if (newTabParent) {
     MOZ_ASSERT(XRE_IsParentProcess());
-    if (aAction.mFocusChange == InputContextAction::MENU_GOT_PSEUDO_FOCUS ||
-        aAction.mFocusChange == InputContextAction::MENU_LOST_PSEUDO_FOCUS) {
-      // XXX When menu keyboard listener is being uninstalled, IME state needs
-      //     to be restored by the child process asynchronously.  Therefore,
-      //     some key events which are fired immediately after closing menu
-      //     may not be handled by IME.
-      Unused << newTabParent->
-        SendMenuKeyboardListenerInstalled(sInstalledMenuKeyboardListener);
-      setIMEState = sInstalledMenuKeyboardListener;
+    if (aAction.mFocusChange == InputContextAction::MENU_GOT_PSEUDO_FOCUS) {
+      // If menu keyboard listener is installed, we need to disable IME now.
+      setIMEState = true;
+    } else if (aAction.mFocusChange ==
+                 InputContextAction::MENU_LOST_PSEUDO_FOCUS) {
+      // If menu keyboard listener is uninstalled, we need to restore
+      // input context which was set by the remote process.  However, if
+      // the remote process hasn't been set input context yet, we need to
+      // wait next SetInputContextForChildProcess() call.
+      if (HasActiveChildSetInputContext()) {
+        setIMEState = true;
+        newState = sActiveChildInputContext.mIMEState;
+      } else {
+        setIMEState = false;
+      }
     } else if (focusActuallyChanging) {
       InputContext context = newWidget->GetInputContext();
       if (context.mIMEState.mEnabled == IMEState::DISABLED &&
           context.mOrigin == InputContext::ORIGIN_CONTENT) {
         setIMEState = false;
         MOZ_LOG(sISMLog, LogLevel::Debug,
           ("  OnChangeFocusInternal(), doesn't set IME "
            "state because focused element (or document) is in a child process "
            "and the IME state is already disabled by a remote process"));
       } else {
+        // When new remote process gets focus, we should forget input context
+        // coming from old focused remote process.
+        ResetActiveChildInputContext();
         MOZ_LOG(sISMLog, LogLevel::Debug,
           ("  OnChangeFocusInternal(), will disable IME "
            "until new focused element (or document) in the child process "
            "will get focus actually"));
       }
     } else if (newWidget->GetInputContext().mOrigin !=
                  InputContext::ORIGIN_MAIN) {
       // When focus is NOT changed actually, we shouldn't set IME state if
@@ -625,16 +636,20 @@ IMEStateManager::OnChangeFocusInternal(n
       // composition.  Then, we shouldn't commit the composition with making
       // IME state disabled.
       setIMEState = false;
       MOZ_LOG(sISMLog, LogLevel::Debug,
         ("  OnChangeFocusInternal(), doesn't set IME "
          "state because focused element (or document) is already in the child "
          "process"));
     }
+  } else {
+    // When this process gets focus, we should forget input context coming
+    // from remote process.
+    ResetActiveChildInputContext();
   }
 
   if (setIMEState) {
     if (!focusActuallyChanging) {
       // actual focus isn't changing, but if IME enabled state is changing,
       // we should do it.
       InputContext context = newWidget->GetInputContext();
       if (context.mIMEState.mEnabled == newState.mEnabled) {
@@ -655,19 +670,27 @@ IMEStateManager::OnChangeFocusInternal(n
       // If aContent isn't null or aContent is null but editable, somebody gets
       // focus.
       bool gotFocus = aContent || (newState.mEnabled == IMEState::ENABLED);
       aAction.mFocusChange =
         gotFocus ? InputContextAction::GOT_FOCUS :
                    InputContextAction::LOST_FOCUS;
     }
 
-    // Update IME state for new focus widget
-    SetIMEState(newState, aPresContext, aContent, newWidget, aAction,
-                newTabParent ? InputContext::ORIGIN_CONTENT : sOrigin);
+    if (newTabParent && HasActiveChildSetInputContext() &&
+        aAction.mFocusChange == InputContextAction::MENU_LOST_PSEUDO_FOCUS) {
+      // Restore the input context in the active remote process when
+      // menu keyboard listener is uninstalled and active remote tab has
+      // focus.
+      SetInputContext(newWidget, sActiveChildInputContext, aAction);
+    } else {
+      // Update IME state for new focus widget
+      SetIMEState(newState, aPresContext, aContent, newWidget, aAction,
+                  newTabParent ? InputContext::ORIGIN_CONTENT : sOrigin);
+    }
   }
 
   sActiveTabParent = newTabParent;
   sPresContext = aPresContext;
   sContent = aContent;
 
   // Don't call CreateIMEContentObserver() here except when a plugin gets
   // focus because it will be called from the focus event handler of focused
@@ -687,18 +710,29 @@ IMEStateManager::OnChangeFocusInternal(n
 }
 
 // static
 void
 IMEStateManager::OnInstalledMenuKeyboardListener(bool aInstalling)
 {
   MOZ_LOG(sISMLog, LogLevel::Info,
     ("OnInstalledMenuKeyboardListener(aInstalling=%s), "
-     "sInstalledMenuKeyboardListener=%s",
-     GetBoolName(aInstalling), GetBoolName(sInstalledMenuKeyboardListener)));
+     "sInstalledMenuKeyboardListener=%s, sActiveTabParent=0x%p, "
+     "sActiveChildInputContext={ mIMEState={ mEnabled=%s, mOpen=%s }, "
+     "mHTMLInputType=\"%s\", mHTMLInputInputmode=\"%s\", mActionHint=\"%s\", "
+     "mInPrivateBrowsing=%s }",
+     GetBoolName(aInstalling),
+     GetBoolName(sInstalledMenuKeyboardListener),
+     sActiveTabParent.get(),
+     GetIMEStateEnabledName(sActiveChildInputContext.mIMEState.mEnabled),
+     GetIMEStateSetOpenName(sActiveChildInputContext.mIMEState.mOpen),
+     NS_ConvertUTF16toUTF8(sActiveChildInputContext.mHTMLInputType).get(),
+     NS_ConvertUTF16toUTF8(sActiveChildInputContext.mHTMLInputInputmode).get(),
+     NS_ConvertUTF16toUTF8(sActiveChildInputContext.mActionHint).get(),
+     GetBoolName(sActiveChildInputContext.mInPrivateBrowsing)));
 
   sInstalledMenuKeyboardListener = aInstalling;
 
   InputContextAction action(InputContextAction::CAUSE_UNKNOWN,
     aInstalling ? InputContextAction::MENU_GOT_PSEUDO_FOCUS :
                   InputContextAction::MENU_LOST_PSEUDO_FOCUS);
   OnChangeFocusInternal(sPresContext, sContent, action);
 }
@@ -1127,39 +1161,53 @@ MayBeIMEUnawareWebApp(nsINode* aNode)
     aNode = aNode->GetParentNode();
   }
 
   return haveKeyEventsListener;
 }
 
 // static
 void
+IMEStateManager::ResetActiveChildInputContext()
+{
+  sActiveChildInputContext.mIMEState.mEnabled = IMEState::UNKNOWN;
+}
+
+// static
+bool
+IMEStateManager::HasActiveChildSetInputContext()
+{
+  return sActiveChildInputContext.mIMEState.mEnabled != IMEState::UNKNOWN;
+}
+
+// static
+void
 IMEStateManager::SetInputContextForChildProcess(
                    TabParent* aTabParent,
                    const InputContext& aInputContext,
                    const InputContextAction& aAction)
 {
   MOZ_LOG(sISMLog, LogLevel::Info,
     ("SetInputContextForChildProcess(aTabParent=0x%p, "
      "aInputContext={ mIMEState={ mEnabled=%s, mOpen=%s }, "
      "mHTMLInputType=\"%s\", mHTMLInputInputmode=\"%s\", mActionHint=\"%s\", "
      "mInPrivateBrowsing=%s }, aAction={ mCause=%s, mAction=%s }), "
      "sPresContext=0x%p (available: %s), sWidget=0x%p (available: %s), "
-     "sActiveTabParent=0x%p",
+     "sActiveTabParent=0x%p, sInstalledMenuKeyboardListener=%s",
      aTabParent, GetIMEStateEnabledName(aInputContext.mIMEState.mEnabled),
      GetIMEStateSetOpenName(aInputContext.mIMEState.mOpen),
      NS_ConvertUTF16toUTF8(aInputContext.mHTMLInputType).get(),
      NS_ConvertUTF16toUTF8(aInputContext.mHTMLInputInputmode).get(),
      NS_ConvertUTF16toUTF8(aInputContext.mActionHint).get(),
      GetBoolName(aInputContext.mInPrivateBrowsing),
      GetActionCauseName(aAction.mCause),
      GetActionFocusChangeName(aAction.mFocusChange),
      sPresContext.get(), GetBoolName(CanHandleWith(sPresContext)),
      sWidget, GetBoolName(sWidget && !sWidget->Destroyed()),
-     sActiveTabParent.get()));
+     sActiveTabParent.get(), GetBoolName(sInstalledMenuKeyboardListener)));
 
   if (aTabParent != sActiveTabParent) {
     MOZ_LOG(sISMLog, LogLevel::Error,
       ("  SetInputContextForChildProcess(), FAILED, "
        "because non-focused tab parent tries to set input context"));
     return;
   }
 
@@ -1178,16 +1226,30 @@ IMEStateManager::SetInputContextForChild
   }
 
   nsCOMPtr<nsIWidget> widget(sWidget);
 
   MOZ_ASSERT(!sPresContext->GetRootWidget() ||
              sPresContext->GetRootWidget() == widget);
   MOZ_ASSERT(aInputContext.mOrigin == InputContext::ORIGIN_CONTENT);
 
+  sActiveChildInputContext = aInputContext;
+  MOZ_ASSERT(HasActiveChildSetInputContext());
+
+  // If input context is changed in remote process while menu keyboard listener
+  // is installed, this process shouldn't set input context now.  When it's
+  // uninstalled, input context should be restored from
+  // sActiveChildInputContext.
+  if (sInstalledMenuKeyboardListener) {
+    MOZ_LOG(sISMLog, LogLevel::Info,
+      ("  SetInputContextForChildProcess(), waiting to set input context "
+       "until menu keyboard listener is uninstalled"));
+    return;
+  }
+
   SetInputContext(widget, aInputContext, aAction);
 }
 
 // static
 void
 IMEStateManager::SetIMEState(const IMEState& aState,
                              nsPresContext* aPresContext,
                              nsIContent* aContent,
--- a/dom/events/IMEStateManager.h
+++ b/dom/events/IMEStateManager.h
@@ -128,16 +128,27 @@ public:
   /**
    * OnChangeFocus() should be called when focused content is changed or
    * IME enabled state is changed.  If nobody has focus, set both aPresContext
    * and aContent nullptr.  E.g., all windows are deactivated.
    */
   static nsresult OnChangeFocus(nsPresContext* aPresContext,
                                 nsIContent* aContent,
                                 InputContextAction::Cause aCause);
+
+  /**
+   * OnInstalledMenuKeyboardListener() is called when menu keyboard listener
+   * is installed or uninstalled in the process.  So, even if menu keyboard
+   * listener was installed in chrome process, this won't be called in content
+   * processes.
+   *
+   * @param aInstalling     true if menu keyboard listener is installed.
+   *                        Otherwise, i.e., menu keyboard listener is
+   *                        uninstalled, false.
+   */
   static void OnInstalledMenuKeyboardListener(bool aInstalling);
 
   // These two methods manage focus and selection/text observers.
   // They are separate from OnChangeFocus above because this offers finer
   // control compared to having the two methods incorporated into OnChangeFocus
 
   // Get the focused editor's selection and root
   static nsresult GetFocusSelectionAndRoot(nsISelection** aSel,
@@ -287,16 +298,29 @@ protected:
 
   static nsIContent* GetRootContent(nsPresContext* aPresContext);
 
   /**
    * CanHandleWith() returns false if aPresContext is nullptr or it's destroyed.
    */
   static bool CanHandleWith(nsPresContext* aPresContext);
 
+  /**
+   * ResetActiveChildInputContext() resets sActiveChildInputContext.
+   * So, HasActiveChildSetInputContext() will return false until a remote
+   * process gets focus and set input context.
+   */
+  static void ResetActiveChildInputContext();
+
+  /**
+   * HasActiveChildSetInputContext() returns true if a remote tab has focus
+   * and it has already set input context.  Otherwise, returns false.
+   */
+  static bool HasActiveChildSetInputContext();
+
   // sContent and sPresContext are the focused content and PresContext.  If a
   // document has focus but there is no focused element, sContent may be
   // nullptr.
   static StaticRefPtr<nsIContent> sContent;
   static StaticRefPtr<nsPresContext> sPresContext;
   // sWidget is cache for the root widget of sPresContext.  Even afer
   // sPresContext has gone, we need to clean up some IME state on the widget
   // if the widget is available.
@@ -319,20 +343,30 @@ protected:
   // When you get an item of this array and use it, please be careful.
   // The instances in this array can be destroyed automatically if you do
   // something to cause committing or canceling the composition.
   static TextCompositionArray* sTextCompositions;
 
   // Origin type of current process.
   static InputContext::Origin sOrigin;
 
-  static bool           sInstalledMenuKeyboardListener;
-  static bool           sIsGettingNewIMEState;
-  static bool           sCheckForIMEUnawareWebApps;
-  static bool           sInputModeSupported;
+  // sActiveChildInputContext is valid only when sActiveTabParent is not
+  // nullptr.  This stores last information of input context in the remote
+  // process of sActiveTabParent.  I.e., they are set when
+  // SetInputContextForChildProcess() is called.  This is necessary for
+  // restoring IME state when menu keyboard listener is uninstalled.
+  static InputContext sActiveChildInputContext;
+
+  // sInstalledMenuKeyboardListener is true if menu keyboard listener is
+  // installed in the process.
+  static bool sInstalledMenuKeyboardListener;
+
+  static bool sIsGettingNewIMEState;
+  static bool sCheckForIMEUnawareWebApps;
+  static bool sInputModeSupported;
 
   class MOZ_STACK_CLASS GettingNewIMEStateBlocker final
   {
   public:
     GettingNewIMEStateBlocker()
       : mOldValue(IMEStateManager::sIsGettingNewIMEState)
     {
       IMEStateManager::sIsGettingNewIMEState = true;
--- a/dom/ipc/PBrowser.ipdl
+++ b/dom/ipc/PBrowser.ipdl
@@ -628,22 +628,16 @@ child:
 
     /**
      * StopIMEStateManagement() is called when the process loses focus and
      * should stop managing IME state.
      */
     async StopIMEStateManagement();
 
     /**
-     * MenuKeyboardListenerInstalled() is called when menu keyboard listener
-     * is installed in the parent process.
-     */
-    async MenuKeyboardListenerInstalled(bool aInstalled);
-
-    /**
      * @see nsIDOMWindowUtils sendMouseEvent.
      */
     async MouseEvent(nsString aType,
                      float aX,
                      float aY,
                      int32_t aButton,
                      int32_t aClickCount,
                      int32_t aModifiers,
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -1519,23 +1519,16 @@ TabChild::RecvSetKeyboardIndicators(cons
 mozilla::ipc::IPCResult
 TabChild::RecvStopIMEStateManagement()
 {
   IMEStateManager::StopIMEStateManagement();
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
-TabChild::RecvMenuKeyboardListenerInstalled(const bool& aInstalled)
-{
-  IMEStateManager::OnInstalledMenuKeyboardListener(aInstalled);
-  return IPC_OK();
-}
-
-mozilla::ipc::IPCResult
 TabChild::RecvNotifyAttachGroupedSHistory(const uint32_t& aOffset)
 {
   // nsISHistory uses int32_t
   if (NS_WARN_IF(aOffset > INT32_MAX)) {
     return IPC_FAIL_NO_REASON(this);
   }
 
   nsCOMPtr<nsISHistory> shistory = GetRelatedSHistory();
--- a/dom/ipc/TabChild.h
+++ b/dom/ipc/TabChild.h
@@ -789,19 +789,16 @@ protected:
 
   virtual mozilla::ipc::IPCResult RecvParentActivated(const bool& aActivated) override;
 
   virtual mozilla::ipc::IPCResult RecvSetKeyboardIndicators(const UIStateChangeType& aShowAccelerators,
                                                             const UIStateChangeType& aShowFocusRings) override;
 
   virtual mozilla::ipc::IPCResult RecvStopIMEStateManagement() override;
 
-  virtual mozilla::ipc::IPCResult RecvMenuKeyboardListenerInstalled(
-    const bool& aInstalled) override;
-
   virtual mozilla::ipc::IPCResult RecvNotifyAttachGroupedSHistory(const uint32_t& aOffset) override;
 
   virtual mozilla::ipc::IPCResult RecvNotifyPartialSHistoryActive(const uint32_t& aGlobalLength,
                                                                   const uint32_t& aTargetLocalIndex) override;
 
   virtual mozilla::ipc::IPCResult RecvNotifyPartialSHistoryDeactive() override;
 
   virtual mozilla::ipc::IPCResult RecvAwaitLargeAlloc() override;