Bug 1251063 PuppetWidget should cache InputContext which is set with SetInputContext() and use it in GetInputContext() only when it is the widget which has active input context in the process r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Sat, 28 May 2016 11:27:56 +0900
changeset 372607 2207eec072023fbca2103605bf1b9bdcfbbbbb55
parent 371406 b0096c5c727749ad3e79cbdf20d2e96bd179c213
child 522190 4ac0661f824fb2475ffae3a9b39f139cbbe48c9e
push id19537
push usermasayuki@d-toybox.com
push dateSat, 28 May 2016 23:10:55 +0000
reviewerssmaug
bugs1251063
milestone49.0a1
Bug 1251063 PuppetWidget should cache InputContext which is set with SetInputContext() and use it in GetInputContext() only when it is the widget which has active input context in the process r?smaug PuppetWidget::GetInputContext() needs to communicate with its parent process with synchronous IPC. This is very expensive for focus move. Currently, IMEStateManager uses nsIWidget::GetInputContext() only for checking the IME enabled state. Therefore, it's enough to cache input context when nsIWidget::SetInputContext() is called. Then, we can avoid to communicate with synchronous IPC with PuppetWidget::GetInputContext() in most cases. This patch makes IMEStateManager stores the last widget which sets input context. When PuppetWidget uses its input context cache, it should check if it is the last widget to set input context with IMEStateManager since an input context may be shared with other widgets and another one may have update the input context. I.e., PuppetWidget's input context cache may be already outdated after IMEStateManager sets input context with another widget. This patch gives up to support retrieving IME open state from child process. However, perhaps, this is not necessary for everybody including add-on developers because the only user of IME open state in child process is nsIDOMWindowUtils. So, add-ons can send IME open state from chrome process instead. If this decision is wrong, unfortunately, we should support it again in another bug. It's easy to support with creating another nsIWidget::GetInputContext() or adding additional argument to it. MozReview-Commit-ID: B2d2CCTsPKj
dom/events/IMEStateManager.cpp
dom/events/IMEStateManager.h
widget/IMEData.h
widget/PuppetWidget.cpp
--- a/dom/events/IMEStateManager.cpp
+++ b/dom/events/IMEStateManager.cpp
@@ -159,17 +159,18 @@ GetNotifyIMEMessageName(IMEMessage aMess
       return "REQUEST_TO_CANCEL_COMPOSITION";
     default:
       return "unacceptable IME notification message";
   }
 }
 
 StaticRefPtr<nsIContent> IMEStateManager::sContent;
 nsPresContext* IMEStateManager::sPresContext = nullptr;
-nsIWidget* IMEStateManager::sFocusedIMEWidget;
+nsIWidget* IMEStateManager::sFocusedIMEWidget = nullptr;
+nsIWidget* IMEStateManager::sActiveInputContextWidget = nullptr;
 StaticRefPtr<TabParent> IMEStateManager::sActiveTabParent;
 StaticRefPtr<IMEContentObserver> IMEStateManager::sActiveIMEContentObserver;
 TextCompositionArray* IMEStateManager::sTextCompositions = nullptr;
 bool IMEStateManager::sInstalledMenuKeyboardListener = false;
 bool IMEStateManager::sIsGettingNewIMEState = false;
 bool IMEStateManager::sCheckForIMEUnawareWebApps = false;
 bool IMEStateManager::sRemoteHasFocus = false;
 
@@ -217,31 +218,35 @@ IMEStateManager::OnTabParentDestroying(T
 
 // static
 void
 IMEStateManager::WidgetDestroyed(nsIWidget* aWidget)
 {
   if (sFocusedIMEWidget == aWidget) {
     sFocusedIMEWidget = nullptr;
   }
+  if (sActiveInputContextWidget == aWidget) {
+    sActiveInputContextWidget = nullptr;
+  }
 }
 
 // static
 void
 IMEStateManager::StopIMEStateManagement()
 {
   MOZ_LOG(sISMLog, LogLevel::Info,
     ("ISM: IMEStateManager::StopIMEStateManagement()"));
 
   // NOTE: Don't set input context from here since this has already lost
   //       the rights to change input context.
 
   if (sTextCompositions && sPresContext) {
     NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, sPresContext);
   }
+  sActiveInputContextWidget = nullptr;
   sPresContext = nullptr;
   sContent = nullptr;
   sActiveTabParent = nullptr;
   DestroyIMEContentObserver();
 }
 
 // static
 nsresult
@@ -999,18 +1004,16 @@ IMEStateManager::SetIMEState(const IMESt
      GetIMEStateEnabledName(aState.mEnabled),
      GetIMEStateSetOpenName(aState.mOpen), aContent,
      TabParent::GetFrom(aContent), aWidget,
      GetActionCauseName(aAction.mCause),
      GetActionFocusChangeName(aAction.mFocusChange)));
 
   NS_ENSURE_TRUE_VOID(aWidget);
 
-  InputContext oldContext = aWidget->GetInputContext();
-
   InputContext context;
   context.mIMEState = aState;
   context.mMayBeIMEUnaware = context.mIMEState.IsEditable() &&
     sCheckForIMEUnawareWebApps && MayBeIMEUnawareWebApp(aContent);
 
   if (aContent &&
       aContent->IsAnyOfHTMLElements(nsGkAtoms::input, nsGkAtoms::textarea)) {
     if (!aContent->IsHTMLElement(nsGkAtoms::textarea)) {
@@ -1103,23 +1106,25 @@ IMEStateManager::SetInputContext(nsIWidg
      NS_ConvertUTF16toUTF8(aInputContext.mHTMLInputInputmode).get(),
      NS_ConvertUTF16toUTF8(aInputContext.mActionHint).get(),
      GetActionCauseName(aAction.mCause),
      GetActionFocusChangeName(aAction.mFocusChange),
      sActiveTabParent.get()));
 
   MOZ_RELEASE_ASSERT(aWidget);
 
-  InputContext oldContext = aWidget->GetInputContext();
+  aWidget->SetInputContext(aInputContext, aAction);
+  sActiveInputContextWidget = aWidget;
 
-  aWidget->SetInputContext(aInputContext, aAction);
-  if (oldContext.mIMEState.mEnabled == aInputContext.mIMEState.mEnabled) {
-    return;
-  }
+  // Don't compare with old IME enabled state for reducing the count of
+  // notifying observers since in a remote process, nsIWidget::GetInputContext()
+  // call here may cause synchronous IPC, it's much more expensive than
+  // notifying observes.
 
+  // XXX Looks like nobody is observing this.
   nsContentUtils::AddScriptRunner(
     new IMEEnabledStateChangedEvent(aInputContext.mIMEState.mEnabled));
 }
 
 // static
 void
 IMEStateManager::EnsureTextCompositionArray()
 {
--- a/dom/events/IMEStateManager.h
+++ b/dom/events/IMEStateManager.h
@@ -65,16 +65,30 @@ public:
   static void OnTabParentDestroying(TabParent* aTabParent);
 
   /**
    * Called when aWidget is being deleted.
    */
   static void WidgetDestroyed(nsIWidget* aWidget);
 
   /**
+   * GetWidgetForActiveInputContext() returns a widget which IMEStateManager
+   * is managing input context with.  If a widget instance needs to cache
+   * the last input context for nsIWidget::GetInputContext() or something,
+   * it should check if its cache is valid with this method before using it
+   * because if this method returns another instance, it means that
+   * IMEStateManager may have already changed shared input context via the
+   * widget.
+   */
+  static nsIWidget* GetWidgetForActiveInputContext()
+  {
+    return sActiveInputContextWidget;
+  }
+
+  /**
    * SetIMEContextForChildProcess() is called when aTabParent receives
    * SetInputContext() from the remote process.
    */
   static void SetInputContextForChildProcess(TabParent* aTabParent,
                                              const InputContext& aInputContext,
                                              const InputContextAction& aAction);
 
   /**
@@ -232,16 +246,19 @@ protected:
 
   static bool IsIMEObserverNeeded(const IMEState& aState);
 
   static nsIContent* GetRootContent(nsPresContext* aPresContext);
 
   static StaticRefPtr<nsIContent> sContent;
   static nsPresContext* sPresContext;
   static nsIWidget* sFocusedIMEWidget;
+  // sActiveInputContextWidget is the last widget whose SetInputContext() is
+  // called.
+  static nsIWidget* sActiveInputContextWidget;
   static StaticRefPtr<TabParent> sActiveTabParent;
   // sActiveIMEContentObserver points to the currently active
   // IMEContentObserver.  This is null if there is no focused editor.
   static StaticRefPtr<IMEContentObserver> sActiveIMEContentObserver;
 
   // All active compositions in the process are stored by this array.
   // 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
--- a/widget/IMEData.h
+++ b/widget/IMEData.h
@@ -158,17 +158,22 @@ struct IMEState final
      */
     PASSWORD,
     /**
      * This state is used when a plugin is focused.
      * When a plug-in is focused content, we should send native events
      * directly. Because we don't process some native events, but they may
      * be needed by the plug-in.
      */
-    PLUGIN
+    PLUGIN,
+    /**
+     * 'Unknown' is useful when you cache this enum.  So, this shouldn't be
+     * used with nsIWidget::SetInputContext().
+     */
+    UNKNOWN
   };
   Enabled mEnabled;
 
   /**
    * IME open states the mOpen value of SetInputContext() should be one value of
    * OPEN, CLOSE or DONT_CHANGE_OPEN_STATE.  GetInputContext() should return
    * OPEN, CLOSE or OPEN_STATE_NOT_SUPPORTED.
    */
--- a/widget/PuppetWidget.cpp
+++ b/widget/PuppetWidget.cpp
@@ -82,16 +82,19 @@ PuppetWidget::PuppetWidget(TabChild* aTa
   , mCursorHotspotY(0)
   , mNativeKeyCommandsValid(false)
 {
   MOZ_COUNT_CTOR(PuppetWidget);
 
   mSingleLineCommands.SetCapacity(4);
   mMultiLineCommands.SetCapacity(4);
   mRichTextCommands.SetCapacity(4);
+
+  // Setting 'Unknown' means "not yet cached".
+  mInputContext.mIMEState.mEnabled = IMEState::UNKNOWN;
 }
 
 PuppetWidget::~PuppetWidget()
 {
   MOZ_COUNT_DTOR(PuppetWidget);
 
   Destroy();
 }
@@ -678,16 +681,20 @@ PuppetWidget::DefaultProcOfPluginEvent(c
   mTabChild->SendDefaultProcOfPluginEvent(aEvent);
 }
 
 NS_IMETHODIMP_(void)
 PuppetWidget::SetInputContext(const InputContext& aContext,
                               const InputContextAction& aAction)
 {
   mInputContext = aContext;
+  // Any widget instances cannot cache IME open state because IME open state
+  // can be changed by user but native IME may not notify us of changing the
+  // open state on some platforms.
+  mInputContext.mIMEState.mOpen = IMEState::OPEN_STATE_NOT_SUPPORTED;
 
 #ifndef MOZ_CROSS_PROCESS_IME
   return;
 #endif
 
   if (!mTabChild) {
     return;
   }
@@ -703,20 +710,35 @@ PuppetWidget::SetInputContext(const Inpu
 
 NS_IMETHODIMP_(InputContext)
 PuppetWidget::GetInputContext()
 {
 #ifndef MOZ_CROSS_PROCESS_IME
   return InputContext();
 #endif
 
+  // XXX Currently, we don't support retrieving IME open state from child
+  //     process.
+
+  // When this widget caches input context and currently managed by
+  // IMEStateManager, the cache is valid.  Only in this case, we can
+  // avoid to use synchronous IPC.
+  if (mInputContext.mIMEState.mEnabled != IMEState::UNKNOWN &&
+      IMEStateManager::GetWidgetForActiveInputContext() == this) {
+    return mInputContext;
+  }
+
+  NS_WARNING("PuppetWidget::GetInputContext() needs to retrieve it with IPC");
+
+  // Don't cache InputContext here because this process isn't managing IME
+  // state of the chrome widget.  So, we cannot modify mInputContext when
+  // chrome widget is set to new context.
   InputContext context;
   if (mTabChild) {
     int32_t enabled, open;
-    // TODO: This is too expensive. PuppetWidget should cache IMEState.
     mTabChild->SendGetInputContext(&enabled, &open);
     context.mIMEState.mEnabled = static_cast<IMEState::Enabled>(enabled);
     context.mIMEState.mOpen = static_cast<IMEState::Open>(open);
   }
   return context;
 }
 
 NS_IMETHODIMP_(NativeIMEContext)