Bug 805767 part.1 nsTextStateManager should use nsIWidget::GetIMEUpdatePreference() instead of the result of nsIWidget::OnIMEFocusChange() r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 13 Nov 2012 22:04:44 +0900
changeset 117718 cc77a4ed08bdb8e4c1753aeb62eeda3e501ada78
parent 117717 2543054f04d984b1ffb1afc1541e5a17cde14c86
child 117719 1f4c0a90aa2c39370dde7951d89f31b4eb63f5ec
push idunknown
push userunknown
push dateunknown
reviewerssmaug
bugs805767
milestone19.0a1
Bug 805767 part.1 nsTextStateManager should use nsIWidget::GetIMEUpdatePreference() instead of the result of nsIWidget::OnIMEFocusChange() r=smaug
content/events/src/nsIMEStateManager.cpp
dom/ipc/TabParent.cpp
widget/nsIWidget.h
widget/xpwidgets/PuppetWidget.cpp
widget/xpwidgets/PuppetWidget.h
--- a/content/events/src/nsIMEStateManager.cpp
+++ b/content/events/src/nsIMEStateManager.cpp
@@ -734,23 +734,21 @@ nsTextStateManager::nsTextStateManager(n
   NS_ENSURE_TRUE_VOID(mRootContent);
 
   if (nsIMEStateManager::sIsTestingIME) {
     nsIDocument* doc = aPresContext->Document();
     (new nsAsyncDOMEvent(doc, NS_LITERAL_STRING("MozIMEFocusIn"),
                          false, false))->RunDOMEventWhenSafe();
   }
 
-  nsresult rv = mWidget->OnIMEFocusChange(true);
-  if (rv == NS_ERROR_NOT_IMPLEMENTED) {
-    return;
+  mWidget->OnIMEFocusChange(true);
+
+  if (mWidget->GetIMEUpdatePreference().mWantUpdates) {
+    ObserveEditableNode();
   }
-  NS_ENSURE_SUCCESS_VOID(rv);
-
-  ObserveEditableNode();
 }
 
 void
 nsTextStateManager::ObserveEditableNode()
 {
   MOZ_ASSERT(mSel);
   MOZ_ASSERT(mRootContent);
 
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -514,32 +514,30 @@ TabParent::RecvSetBackgroundColor(const 
 }
 
 bool
 TabParent::RecvNotifyIMEFocus(const bool& aFocus,
                               nsIMEUpdatePreference* aPreference,
                               uint32_t* aSeqno)
 {
   nsCOMPtr<nsIWidget> widget = GetWidget();
-  if (!widget)
+  if (!widget) {
+    aPreference->mWantUpdates = false;
+    aPreference->mWantHints = false;
     return true;
+  }
 
   *aSeqno = mIMESeqno;
   mIMETabParent = aFocus ? this : nullptr;
   mIMESelectionAnchor = 0;
   mIMESelectionFocus = 0;
-  nsresult rv = widget->OnIMEFocusChange(aFocus);
+  widget->OnIMEFocusChange(aFocus);
 
   if (aFocus) {
-    if (NS_SUCCEEDED(rv)) {
-      *aPreference = widget->GetIMEUpdatePreference();
-    } else {
-      aPreference->mWantUpdates = false;
-      aPreference->mWantHints = false;
-    }
+    *aPreference = widget->GetIMEUpdatePreference();
   } else {
     mIMECacheText.Truncate(0);
   }
   return true;
 }
 
 bool
 TabParent::RecvNotifyIMETextChange(const uint32_t& aStart,
--- a/widget/nsIWidget.h
+++ b/widget/nsIWidget.h
@@ -181,22 +181,22 @@ enum nsTopLevelWidgetZPlacement { // for
  * After the current process resumes from being suspended, this topic is
  * notified.
  */
 #define NS_WIDGET_RESUME_PROCESS_OBSERVER_TOPIC "resume_process_notification"
 
 /**
  * Preference for receiving IME updates
  *
- * If mWantUpdates is true, PuppetWidget will forward
- * nsIWidget::OnIMETextChange and nsIWidget::OnIMESelectionChange to the chrome
- * process. This incurs overhead from observers and IPDL. If the IME
- * implementation on a particular platform doesn't care about OnIMETextChange
- * and OnIMESelectionChange from content processes, they should set
- * mWantUpdates to false to avoid these overheads.
+ * If mWantUpdates is true, nsTextStateManager will observe text change and
+ * selection change and call nsIWidget::OnIMETextChange() and
+ * nsIWidget::OnIMESelectionChange(). The observing cost is very expensive.
+ * If the IME implementation on a particular platform doesn't care about
+ * OnIMETextChange and OnIMESelectionChange, they should set mWantUpdates to
+ * false to avoid the cost.
  *
  * If mWantHints is true, PuppetWidget will forward the content of text fields
  * to the chrome process to be cached. This way we return the cached content
  * during query events. (see comments in bug 583976). This only makes sense
  * for IME implementations that do use query events, otherwise there's a
  * significant overhead. Platforms that don't use query events should set
  * mWantHints to false.
  */
--- a/widget/xpwidgets/PuppetWidget.cpp
+++ b/widget/xpwidgets/PuppetWidget.cpp
@@ -424,26 +424,31 @@ PuppetWidget::OnIMEFocusChange(bool aFoc
 
   uint32_t chromeSeqno;
   mIMEPreference.mWantUpdates = false;
   mIMEPreference.mWantHints = false;
   if (!mTabChild->SendNotifyIMEFocus(aFocus, &mIMEPreference, &chromeSeqno))
     return NS_ERROR_FAILURE;
 
   if (aFocus) {
-    if (!mIMEPreference.mWantUpdates && !mIMEPreference.mWantHints)
-      // call OnIMEFocusChange on blur but no other updates
-      return NS_ERROR_NOT_IMPLEMENTED;
-    OnIMESelectionChange(); // Update selection
+    if (mIMEPreference.mWantUpdates && mIMEPreference.mWantHints) {
+      OnIMESelectionChange(); // Update selection
+    }
   } else {
     mIMELastBlurSeqno = chromeSeqno;
   }
   return NS_OK;
 }
 
+nsIMEUpdatePreference
+PuppetWidget::GetIMEUpdatePreference()
+{
+  return mIMEPreference;
+}
+
 NS_IMETHODIMP
 PuppetWidget::OnIMETextChange(uint32_t aStart, uint32_t aEnd, uint32_t aNewEnd)
 {
   if (!mTabChild)
     return NS_ERROR_FAILURE;
 
   if (mIMEPreference.mWantHints) {
     nsEventStatus status;
--- a/widget/xpwidgets/PuppetWidget.h
+++ b/widget/xpwidgets/PuppetWidget.h
@@ -153,16 +153,17 @@ public:
   NS_IMETHOD_(void) SetInputContext(const InputContext& aContext,
                                     const InputContextAction& aAction);
   NS_IMETHOD_(InputContext) GetInputContext();
   NS_IMETHOD CancelComposition();
   NS_IMETHOD OnIMEFocusChange(bool aFocus);
   NS_IMETHOD OnIMETextChange(uint32_t aOffset, uint32_t aEnd,
                              uint32_t aNewEnd);
   NS_IMETHOD OnIMESelectionChange(void);
+  virtual nsIMEUpdatePreference GetIMEUpdatePreference();
 
   NS_IMETHOD SetCursor(nsCursor aCursor);
   NS_IMETHOD SetCursor(imgIContainer* aCursor,
                        uint32_t aHotspotX, uint32_t aHotspotY)
   {
     return nsBaseWidget::SetCursor(aCursor, aHotspotX, aHotspotY);
   }