From 595be5cb4f482f2a4740763b01df7672ccaa215b Mon Sep 17 00:00:00 2001 From: Gabor Krizsanits Date: Fri, 20 Feb 2015 17:47:29 +0100 Subject: [PATCH] Bug 1072350 - Removing CPOWs used by Find events. --- .../firefox/content/PdfStreamConverter.jsm | 84 +++++---- .../firefox/content/PdfjsChromeUtils.jsm | 169 +++++++++--------- 2 files changed, 127 insertions(+), 126 deletions(-) diff --git a/extensions/firefox/content/PdfStreamConverter.jsm b/extensions/firefox/content/PdfStreamConverter.jsm index 8ca76c995..07eb7a079 100644 --- a/extensions/firefox/content/PdfStreamConverter.jsm +++ b/extensions/firefox/content/PdfStreamConverter.jsm @@ -72,7 +72,7 @@ function getChromeWindow(domWindow) { function getFindBar(domWindow) { if (PdfjsContentUtils.isRemote) { - return PdfjsContentUtils.getFindBar(domWindow); + throw new Error('FindBar is not accessible from the content process.'); } var browser = getContainingBrowser(domWindow); try { @@ -371,7 +371,13 @@ ChromeActions.prototype = { if (this.domWindow.frameElement !== null) { return false; } - // ... and when the new find events code exists. + + // ... and we are in a child process + if (PdfjsContentUtils.isRemote) { + return true; + } + + // ... or when the new find events code exists. var findBar = getFindBar(this.domWindow); return findBar && ('updateControlState' in findBar); }, @@ -473,7 +479,15 @@ ChromeActions.prototype = { (findPreviousType !== 'undefined' && findPreviousType !== 'boolean')) { return; } - getFindBar(this.domWindow).updateControlState(result, findPrevious); + + var winmm = this.domWindow.QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIDocShell) + .sameTypeRootTreeItem + .QueryInterface(Ci.nsIDocShell) + .QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIContentFrameMessageManager); + + winmm.sendAsyncMessage('PDFJS:Parent:updateControlState', data); }, setPreferences: function(prefs, sendResponse) { var defaultBranch = Services.prefs.getDefaultBranch(PREF_PREFIX + '.'); @@ -778,14 +792,14 @@ RequestListener.prototype.receive = function(event) { // Forwards events from the eventElement to the contentWindow only if the // content window matches the currently selected browser window. -function FindEventManager(eventElement, contentWindow, chromeWindow) { - this.types = ['find', - 'findagain', - 'findhighlightallchange', - 'findcasesensitivitychange']; - this.chromeWindow = chromeWindow; +function FindEventManager(contentWindow) { this.contentWindow = contentWindow; - this.eventElement = eventElement; + this.winmm = contentWindow.QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIDocShell) + .sameTypeRootTreeItem + .QueryInterface(Ci.nsIDocShell) + .QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIContentFrameMessageManager); } FindEventManager.prototype.bind = function() { @@ -795,41 +809,27 @@ FindEventManager.prototype.bind = function() { }.bind(this); this.contentWindow.addEventListener('unload', unload); - for (var i = 0; i < this.types.length; i++) { - var type = this.types[i]; - this.eventElement.addEventListener(type, this, true); - } + // We cannot directly attach listeners to for the find events + // since the FindBar is in the parent process. Instead we're + // asking the PdfjsChromeUtils to do it for us and forward + // all the find events to us. + this.winmm.sendAsyncMessage('PDFJS:Parent:addEventListener'); + this.winmm.addMessageListener('PDFJS:Child:handleEvent', this); }; -FindEventManager.prototype.handleEvent = function(e) { - var chromeWindow = this.chromeWindow; +FindEventManager.prototype.receiveMessage = function(msg) { + var detail = msg.data.detail; + var type = msg.data.type; var contentWindow = this.contentWindow; - // Only forward the events if they are for our dom window. - if (chromeWindow.gBrowser.selectedBrowser.contentWindow === contentWindow) { - var detail = { - query: e.detail.query, - caseSensitive: e.detail.caseSensitive, - highlightAll: e.detail.highlightAll, - findPrevious: e.detail.findPrevious - }; - detail = makeContentReadable(detail, contentWindow); - var forward = contentWindow.document.createEvent('CustomEvent'); - forward.initCustomEvent(e.type, true, true, detail); - // Due to restrictions with cpow use, we can't dispatch - // dom events with an urgent message on the stack. So bounce - // this off the main thread to make it async. - Services.tm.mainThread.dispatch(function () { - contentWindow.dispatchEvent(forward); - }, Ci.nsIThread.DISPATCH_NORMAL); - e.preventDefault(); - } + + detail = makeContentReadable(detail, contentWindow); + var forward = contentWindow.document.createEvent('CustomEvent'); + forward.initCustomEvent(type, true, true, detail); + contentWindow.dispatchEvent(forward); }; FindEventManager.prototype.unbind = function() { - for (var i = 0; i < this.types.length; i++) { - var type = this.types[i]; - this.eventElement.removeEventListener(type, this, true); - } + this.winmm.sendAsyncMessage('PDFJS:Parent:removeEventListener'); }; function PdfStreamConverter() { @@ -994,11 +994,7 @@ PdfStreamConverter.prototype = { requestListener.receive(event); }, false, true); if (actions.supportsIntegratedFind()) { - var chromeWindow = getChromeWindow(domWindow); - var findBar = getFindBar(domWindow); - var findEventManager = new FindEventManager(findBar, - domWindow, - chromeWindow); + var findEventManager = new FindEventManager(domWindow); findEventManager.bind(); } listener.onStopRequest(aRequest, context, statusCode); diff --git a/extensions/firefox/content/PdfjsChromeUtils.jsm b/extensions/firefox/content/PdfjsChromeUtils.jsm index 598502dc7..ed4973289 100644 --- a/extensions/firefox/content/PdfjsChromeUtils.jsm +++ b/extensions/firefox/content/PdfjsChromeUtils.jsm @@ -51,6 +51,7 @@ let PdfjsChromeUtils = { */ init: function () { + this._browsers = new Set(); if (!this._ppmm) { // global parent process message manager (PPMM) this._ppmm = Cc['@mozilla.org/parentprocessmessagemanager;1']. @@ -65,10 +66,12 @@ let PdfjsChromeUtils = { // global dom message manager (MMg) this._mmg = Cc['@mozilla.org/globalmessagemanager;1']. getService(Ci.nsIMessageListenerManager); - this._mmg.addMessageListener('PDFJS:Parent:getChromeWindow', this); - this._mmg.addMessageListener('PDFJS:Parent:getFindBar', this); this._mmg.addMessageListener('PDFJS:Parent:displayWarning', this); + this._mmg.addMessageListener('PDFJS:Parent:addEventListener', this); + this._mmg.addMessageListener('PDFJS:Parent:removeEventListener', this); + this._mmg.addMessageListener('PDFJS:Parent:updateControlState', this); + // observer to handle shutdown Services.obs.addObserver(this, 'quit-application', false); } @@ -84,10 +87,12 @@ let PdfjsChromeUtils = { this._ppmm.removeMessageListener('PDFJS:Parent:isDefaultHandlerApp', this); - this._mmg.removeMessageListener('PDFJS:Parent:getChromeWindow', this); - this._mmg.removeMessageListener('PDFJS:Parent:getFindBar', this); this._mmg.removeMessageListener('PDFJS:Parent:displayWarning', this); + this._mmg.removeMessageListener('PDFJS:Parent:addEventListener', this); + this._mmg.removeMessageListener('PDFJS:Parent:removeEventListener', this); + this._mmg.removeMessageListener('PDFJS:Parent:updateControlState', this); + Services.obs.removeObserver(this, 'quit-application', false); this._mmg = null; @@ -146,11 +151,13 @@ let PdfjsChromeUtils = { this._displayWarning(aMsg); break; - // CPOW getters - case 'PDFJS:Parent:getChromeWindow': - return this._getChromeWindow(aMsg); - case 'PDFJS:Parent:getFindBar': - return this._getFindBar(aMsg); + + case 'PDFJS:Parent:updateControlState': + return this._updateControlState(aMsg); + case 'PDFJS:Parent:addEventListener': + return this._addEventListener(aMsg); + case 'PDFJS:Parent:removeEventListener': + return this._removeEventListener(aMsg); } }, @@ -158,24 +165,81 @@ let PdfjsChromeUtils = { * Internal */ - _getChromeWindow: function (aMsg) { - // See the child module, our return result here can't be the element - // since return results don't get auto CPOW'd. + _findbarFromMessage: function(aMsg) { let browser = aMsg.target; - let wrapper = new PdfjsWindowWrapper(browser); - let suitcase = aMsg.objects.suitcase; - suitcase.setChromeWindow(wrapper); - return true; + let tabbrowser = browser.getTabBrowser(); + let tab = tabbrowser.getTabForBrowser(browser); + return tabbrowser.getFindBar(tab); }, - _getFindBar: function (aMsg) { - // We send this over via the window's message manager, so target should - // be the dom window. + _updateControlState: function (aMsg) { + let data = aMsg.data; + this._findbarFromMessage(aMsg) + .updateControlState(data.result, data.findPrevious); + }, + + handleEvent: function(aEvent) { + // We cannot just forward the message as a CPOW without setting up + // __exposedProps__ on it. Instead, let's just create a structured + // cloneable version of the event for performance and for the ease of usage. + let type = aEvent.type; + let detail = { + query: aEvent.detail.query, + caseSensitive: aEvent.detail.caseSensitive, + highlightAll: aEvent.detail.highlightAll, + findPrevious: aEvent.detail.findPrevious + }; + + let chromeWindow = aEvent.target.ownerDocument.defaultView; + let browser = chromeWindow.gBrowser.selectedBrowser; + if (this._browsers.has(browser)) { + // Only forward the events if the selected browser is a registered + // browser. + let mm = browser.messageManager; + mm.sendAsyncMessage('PDFJS:Child:handleEvent', + { type: type, detail: detail }); + aEvent.preventDefault(); + } + }, + + _types: ['find', + 'findagain', + 'findhighlightallchange', + 'findcasesensitivitychange'], + + _addEventListener: function (aMsg) { let browser = aMsg.target; - let wrapper = new PdfjsFindbarWrapper(browser); - let suitcase = aMsg.objects.suitcase; - suitcase.setFindBar(wrapper); - return true; + if (this._browsers.has(browser)) { + throw new Error('FindEventManager was bound 2nd time ' + + 'without unbinding it first.'); + } + + // Since this jsm is global, we need to store all the browsers + // we have to forward the messages for. + this._browsers.add(browser); + + // And we need to start listening to find events. + for (var i = 0; i < this._types.length; i++) { + var type = this._types[i]; + this._findbarFromMessage(aMsg) + .addEventListener(type, this, true); + } + }, + + _removeEventListener: function (aMsg) { + let browser = aMsg.target; + if (!this._browsers.has(browser)) { + throw new Error('FindEventManager was unbound without binding it first.'); + } + + this._browsers.delete(browser); + + // No reason to listen to find events any longer. + for (var i = 0; i < this._types.length; i++) { + var type = this._types[i]; + this._findbarFromMessage(aMsg) + .removeEventListener(type, this, true); + } }, _ensurePreferenceAllowed: function (aPrefName) { @@ -268,62 +332,3 @@ let PdfjsChromeUtils = { } }; -/* - * CPOW security features require chrome objects declare exposed - * properties via __exposedProps__. We don't want to expose things - * directly on the findbar, so we wrap the findbar in a smaller - * object here that supports the features pdf.js needs. - */ -function PdfjsFindbarWrapper(aBrowser) { - let tabbrowser = aBrowser.getTabBrowser(); - let tab; -//#if MOZCENTRAL - tab = tabbrowser.getTabForBrowser(aBrowser); -//#else - if (tabbrowser.getTabForBrowser) { - tab = tabbrowser.getTabForBrowser(aBrowser); - } else { - // _getTabForBrowser is depreciated in Firefox 35, see - // https://bugzilla.mozilla.org/show_bug.cgi?id=1039500. - tab = tabbrowser._getTabForBrowser(aBrowser); - } -//#endif - this._findbar = tabbrowser.getFindBar(tab); -} - -PdfjsFindbarWrapper.prototype = { - __exposedProps__: { - addEventListener: 'r', - removeEventListener: 'r', - updateControlState: 'r', - }, - _findbar: null, - - updateControlState: function (aResult, aFindPrevious) { - this._findbar.updateControlState(aResult, aFindPrevious); - }, - - addEventListener: function (aType, aListener, aUseCapture, aWantsUntrusted) { - this._findbar.addEventListener(aType, aListener, aUseCapture, - aWantsUntrusted); - }, - - removeEventListener: function (aType, aListener, aUseCapture) { - this._findbar.removeEventListener(aType, aListener, aUseCapture); - } -}; - -function PdfjsWindowWrapper(aBrowser) { - this._window = aBrowser.ownerDocument.defaultView; -} - -PdfjsWindowWrapper.prototype = { - __exposedProps__: { - valueOf: 'r', - }, - _window: null, - - valueOf: function () { - return this._window.valueOf(); - } -};