Bug 1175024 - Don't destroy and recreate DOMRequestIpcHelper instance for every InputContext. r=fabrice
authorTim Chien <timdream@gmail.com>
Thu, 02 Jul 2015 23:47:00 +0200
changeset 275851 cd94f7a2b847521e3b9578b336871398597a9eb6
parent 275850 13bedf2492be370e0a27ea58addab68c7fcb75d8
child 275852 0371fcd2e3e439f85c70860fd2ca450d55ac62f5
push id3246
push usergijskruitbosch@gmail.com
push dateTue, 07 Jul 2015 09:06:38 +0000
reviewersfabrice
bugs1175024
milestone42.0a1
Bug 1175024 - Don't destroy and recreate DOMRequestIpcHelper instance for every InputContext. r=fabrice
dom/inputmethod/Keyboard.jsm
dom/inputmethod/MozKeyboard.js
dom/inputmethod/mochitest/test_bug1043828.html
--- a/dom/inputmethod/Keyboard.jsm
+++ b/dom/inputmethod/Keyboard.jsm
@@ -258,17 +258,17 @@ this.Keyboard = {
       case 'Keyboard:SetComposition':
         this.setComposition(msg);
         break;
       case 'Keyboard:EndComposition':
         this.endComposition(msg);
         break;
       case 'Keyboard:Register':
         this._keyboardMM = mm;
-        if (kbID !== null) {
+        if (kbID) {
           // keyboard identifies itself, use its kbID
           // this msg would be async, so no need to return
           this._keyboardID = kbID;
         }else{
           // generate the id for the keyboard
           this._keyboardID = this._nextKeyboardID;
           this._nextKeyboardID++;
           // this msg is sync,
--- a/dom/inputmethod/MozKeyboard.js
+++ b/dom/inputmethod/MozKeyboard.js
@@ -14,84 +14,115 @@ Cu.import("resource://gre/modules/DOMReq
 
 XPCOMUtils.defineLazyServiceGetter(this, "cpmm",
   "@mozilla.org/childprocessmessagemanager;1", "nsISyncMessageSender");
 
 XPCOMUtils.defineLazyServiceGetter(this, "tm",
   "@mozilla.org/thread-manager;1", "nsIThreadManager");
 
 /*
- * A WeakMap to map input method iframe window to its active status and kbID.
+ * A WeakMap to map input method iframe window to
+ * it's active status, kbID, and ipcHelper.
  */
 let WindowMap = {
   // WeakMap of <window, object> pairs.
   _map: null,
 
   /*
+   * Set the object associated to the window and return it.
+   */
+  _getObjForWin: function(win) {
+    if (!this._map) {
+      this._map = new WeakMap();
+    }
+    if (this._map.has(win)) {
+      return this._map.get(win);
+    } else {
+      let obj = {
+        active: false,
+        kbID: undefined,
+        ipcHelper: null
+      };
+      this._map.set(win, obj);
+
+      return obj;
+    }
+  },
+
+  /*
    * Check if the given window is active.
    */
   isActive: function(win) {
     if (!this._map || !win) {
       return false;
     }
 
-    let obj = this._map.get(win);
-    if (obj && 'active' in obj) {
-      return obj.active;
-    }else{
-      return false;
-    }
+    return this._getObjForWin(win).active;
   },
 
   /*
    * Set the active status of the given window.
    */
   setActive: function(win, isActive) {
     if (!win) {
       return;
     }
-    if (!this._map) {
-      this._map = new WeakMap();
-    }
-    if (!this._map.has(win)) {
-      this._map.set(win, {});
-    }
-    this._map.get(win).active = isActive;
+    let obj = this._getObjForWin(win);
+    obj.active = isActive;
   },
 
   /*
-   * Get the keyboard ID (assigned by Keyboard.ksm) of the given window.
+   * Get the keyboard ID (assigned by Keyboard.jsm) of the given window.
    */
   getKbID: function(win) {
     if (!this._map || !win) {
-      return null;
+      return undefined;
     }
 
-    let obj = this._map.get(win);
-    if (obj && 'kbID' in obj) {
-      return obj.kbID;
-    }else{
-      return null;
-    }
+    let obj = this._getObjForWin(win);
+    return obj.kbID;
   },
 
   /*
-   * Set the keyboard ID (assigned by Keyboard.ksm) of the given window.
+   * Set the keyboard ID (assigned by Keyboard.jsm) of the given window.
    */
   setKbID: function(win, kbID) {
     if (!win) {
       return;
     }
-    if (!this._map) {
-      this._map = new WeakMap();
+    let obj = this._getObjForWin(win);
+    obj.kbID = kbID;
+  },
+
+  /*
+   * Get InputContextDOMRequestIpcHelper instance attached to this window.
+   */
+  getInputContextIpcHelper: function(win) {
+    if (!win) {
+      return;
+    }
+    let obj = this._getObjForWin(win);
+    if (!obj.ipcHelper) {
+      obj.ipcHelper = new InputContextDOMRequestIpcHelper(win);
     }
-    if (!this._map.has(win)) {
-      this._map.set(win, {});
+    return obj.ipcHelper;
+  },
+
+  /*
+   * Unset InputContextDOMRequestIpcHelper instance.
+   */
+  unsetInputContextIpcHelper: function(win) {
+    if (!win) {
+      return;
     }
-    this._map.get(win).kbID = kbID;
+    let obj = this._getObjForWin(win);
+    if (!obj.ipcHelper) {
+      return;
+    }
+    obj.ipcHelper = null;
   }
 };
 
 let cpmmSendAsyncMessageWithKbID = function (self, msg, data) {
   data.kbID = WindowMap.getKbID(self._window);
   cpmm.sendAsyncMessage(msg, data);
 };
 
@@ -327,19 +358,19 @@ MozInputMethod.prototype = {
       // a GetContext:Result:OK event, and we can initialize ourselves.
       // Otherwise silently ignored.
 
       // get keyboard ID from Keyboard.jsm,
       // or if we already have it, get it from our map
       // Note: if we need to get it from Keyboard.jsm,
       // we have to use a synchronous message
       var kbID = WindowMap.getKbID(this._window);
-      if (kbID !== null) {
+      if (kbID) {
         cpmmSendAsyncMessageWithKbID(this, 'Keyboard:Register', {});
-      }else{
+      } else {
         let res = cpmm.sendSyncMessage('Keyboard:Register', {});
         WindowMap.setKbID(this._window, res[0]);
       }
 
       cpmmSendAsyncMessageWithKbID(this, 'Keyboard:GetContext', {});
     } else {
       // Deactive current input method.
       cpmmSendAsyncMessageWithKbID(this, 'Keyboard:Unregister', {});
@@ -412,16 +443,71 @@ MozInputMethod.prototype = {
       let resolverId = self.getPromiseResolverId({ resolve: resolve, reject: reject });
       callback(resolverId);
     });
   }
 };
 
  /**
  * ==============================================
+ * InputContextDOMRequestIpcHelper
+ * ==============================================
+ */
+function InputContextDOMRequestIpcHelper(win) {
+  this.initDOMRequestHelper(win,
+    ["Keyboard:GetText:Result:OK",
+     "Keyboard:GetText:Result:Error",
+     "Keyboard:SetSelectionRange:Result:OK",
+     "Keyboard:ReplaceSurroundingText:Result:OK",
+     "Keyboard:SendKey:Result:OK",
+     "Keyboard:SendKey:Result:Error",
+     "Keyboard:SetComposition:Result:OK",
+     "Keyboard:EndComposition:Result:OK",
+     "Keyboard:SequenceError"]);
+}
+
+InputContextDOMRequestIpcHelper.prototype = {
+  __proto__: DOMRequestIpcHelper.prototype,
+  _inputContext: null,
+
+  attachInputContext: function(inputCtx) {
+    if (this._inputContext) {
+      throw new Error("InputContextDOMRequestIpcHelper: detach the context first.");
+    }
+
+    this._inputContext = inputCtx;
+  },
+
+  // Unset ourselves when the window is destroyed.
+  uninit: function() {
+    WindowMap.unsetInputContextIpcHelper(this._window);
+  },
+
+  detachInputContext: function() {
+    // All requests that are still pending need to be invalidated
+    // because the context is no longer valid.
+    this.forEachPromiseResolver(k => {
+      this.takePromiseResolver(k).reject("InputContext got destroyed");
+    });
+
+    this._inputContext = null;
+  },
+
+  receiveMessage: function(msg) {
+    if (!this._inputContext) {
+      dump('InputContextDOMRequestIpcHelper received message without context attached.\n');
+      return;
+    }
+
+    this._inputContext.receiveMessage(msg);
+  }
+};
+
+ /**
+ * ==============================================
  * InputContext
  * ==============================================
  */
 function MozInputContext(ctx) {
   this._context = {
     inputtype: ctx.type,
     inputmode: ctx.inputmode,
     lang: ctx.lang,
@@ -433,78 +519,63 @@ function MozInputContext(ctx) {
     textBeforeCursor: ctx.textBeforeCursor,
     textAfterCursor: ctx.textAfterCursor
   };
 
   this._contextId = ctx.contextId;
 }
 
 MozInputContext.prototype = {
-  __proto__: DOMRequestIpcHelper.prototype,
-
   _window: null,
   _context: null,
   _contextId: -1,
+  _ipcHelper: null,
 
   classID: Components.ID("{1e38633d-d08b-4867-9944-afa5c648adb6}"),
 
   QueryInterface: XPCOMUtils.generateQI([
     Ci.nsIObserver,
     Ci.nsISupportsWeakReference
   ]),
 
   init: function ic_init(win) {
     this._window = win;
-    this._utils = win.QueryInterface(Ci.nsIInterfaceRequestor)
-                     .getInterface(Ci.nsIDOMWindowUtils);
-    this.initDOMRequestHelper(win,
-      ["Keyboard:GetText:Result:OK",
-       "Keyboard:GetText:Result:Error",
-       "Keyboard:SetSelectionRange:Result:OK",
-       "Keyboard:ReplaceSurroundingText:Result:OK",
-       "Keyboard:SendKey:Result:OK",
-       "Keyboard:SendKey:Result:Error",
-       "Keyboard:SetComposition:Result:OK",
-       "Keyboard:EndComposition:Result:OK",
-       "Keyboard:SequenceError"]);
+
+    this._ipcHelper = WindowMap.getInputContextIpcHelper(win);
+    this._ipcHelper.attachInputContext(this);
   },
 
   destroy: function ic_destroy() {
-    let self = this;
-
-    // All requests that are still pending need to be invalidated
-    // because the context is no longer valid.
-    this.forEachPromiseResolver(function(k) {
-      self.takePromiseResolver(k).reject("InputContext got destroyed");
-    });
-    this.destroyDOMRequestHelper();
-
     // A consuming application might still hold a cached version of
     // this object. After destroying all methods will throw because we
     // cannot create new promises anymore, but we still hold
     // (outdated) information in the context. So let's clear that out.
     for (var k in this._context) {
       if (this._context.hasOwnProperty(k)) {
         this._context[k] = null;
       }
     }
 
+    this._ipcHelper.detachInputContext();
+    this._ipcHelper = null;
+
     this._window = null;
   },
 
   receiveMessage: function ic_receiveMessage(msg) {
     if (!msg || !msg.json) {
       dump('InputContext received message without data\n');
       return;
     }
 
     let json = msg.json;
-    let resolver = this.takePromiseResolver(json.requestId);
+    let resolver = this._ipcHelper.takePromiseResolver(json.requestId);
 
     if (!resolver) {
+      dump('InputContext received invalid requestId.\n');
       return;
     }
 
     // Update context first before resolving promise to avoid race condition
     if (json.selectioninfo) {
       this.updateSelectionContext(json.selectioninfo, true);
     }
 
@@ -710,20 +781,20 @@ MozInputContext.prototype = {
         requestId: resolverId,
         text: text || ''
       });
     });
   },
 
   _sendPromise: function(callback) {
     let self = this;
-    return this.createPromise(function(resolve, reject) {
-      let resolverId = self.getPromiseResolverId({ resolve: resolve, reject: reject });
+    return this._ipcHelper.createPromise(function(resolve, reject) {
+      let resolverId = self._ipcHelper.getPromiseResolverId({ resolve: resolve, reject: reject });
       if (!WindowMap.isActive(self._window)) {
-        self.removePromiseResolver(resolverId);
+        self._ipcHelper.removePromiseResolver(resolverId);
         reject('Input method is not active.');
         return;
       }
       callback(resolverId);
     });
   }
 };
 
--- a/dom/inputmethod/mochitest/test_bug1043828.html
+++ b/dom/inputmethod/mochitest/test_bug1043828.html
@@ -27,17 +27,17 @@ function kbFrameScript() {
     var ctx = content.navigator.mozInputMethod.inputcontext;
     if (ctx) {
       var p = ctx.getText();
       p.then(function(){
         sendAsyncMessage('test:InputMethod:getText:Resolve');
       }, function(e){
         sendAsyncMessage('test:InputMethod:getText:Reject');
       });
-    }else{
+    } else {
       dump("Could not get inputcontext") ;
     }
   }
 
   addMessageListener('test:InputMethod:getText:Do', function(){
     tryGetText();
   });
 }
@@ -98,17 +98,17 @@ function runTest() {
         handleEvent: function(){
           keyboardB.removeEventListener('mozbrowserloadend', this);
 
           mmKeyboardB = SpecialPowers.getBrowserFrameMessageManager(keyboardB);
 
           mmKeyboardB.loadFrameScript('data:,(' + kbFrameScript.toString() + ')();', false);
 
           mmKeyboardB.addMessageListener('test:InputMethod:getText:Resolve', function() {
-            ok(true, 'getText() was resolved');
+            info('getText() was resolved');
             inputmethod_cleanup();
           });
 
           mmKeyboardB.addMessageListener('test:InputMethod:getText:Reject', function() {
             ok(false, 'getText() was rejected');
             inputmethod_cleanup();
           });
 
@@ -119,64 +119,68 @@ function runTest() {
       };
 
       keyboardB.addEventListener('mozbrowserloadend', handler);
     });
   }
 
   // STEP 2: Set keyboard A active
   function step2() {
+    info('step2');
     let req = keyboardA.setInputMethodActive(true);
 
     req.onsuccess = function(){
       setTimeout(function(){
         step3();
       }, WAIT_TIME);
     };
 
     req.onerror = function(){
       ok(false, 'setInputMethodActive failed: ' + this.error.name);
       inputmethod_cleanup();
     };
   }
 
   // STEP 3: Set keyboard B active
   function step3() {
+    info('step3');
     let req = keyboardB.setInputMethodActive(true);
 
     req.onsuccess = function(){
       setTimeout(function(){
         step4();
       }, WAIT_TIME);
     };
 
     req.onerror = function(){
       ok(false, 'setInputMethodActive failed: ' + this.error.name);
       inputmethod_cleanup();
     };
   }
 
   // STEP 4: Set keyboard A inactive
   function step4() {
+    info('step4');
     let req = keyboardA.setInputMethodActive(false);
 
     req.onsuccess = function(){
       setTimeout(function(){
         step5();
       }, WAIT_TIME);
     };
 
     req.onerror = function(){
       ok(false, 'setInputMethodActive failed: ' + this.error.name);
       inputmethod_cleanup();
     };
   }
 
   // STEP 5: getText
   function step5() {
+    info('step5');
     mmKeyboardB.sendAsyncMessage('test:InputMethod:getText:Do');
   }
 
   step1();
 }
 
 </script>
 </pre>