Firefox pdf viewer support for e10s (Bug 942707). #5115

yurydelendik · 2014-08-01T16:01:54Z

See https://bugzilla.mozilla.org/show_bug.cgi?id=942707

Snuffleupagus · 2014-08-05T15:26:51Z

extensions/firefox/content/PdfjsChromeUtils.jsm

+let PdfjsChromeUtils = {
+  // For security purposes when running remote, we restrict preferences
+  // content can access.
+  _allowedPrefNames: [


Hard-coding the preference names here seems unfortunate to me, since it unnecessarily increases the maintenance burden; and it's easy for this file and web/default_preferences.js to get out of sync.

I would instead suggest using the preprocessor to include DEFAULT_PREFERENCES in this file, and then compare against those in the code below; for reference see either bootstrap.js#L54 or PdfJs.jsm#63.

yurydelendik · 2014-08-06T23:14:55Z

@jmathies I was able to run it as an extension, however find functionality does not work. Can you look you that.

Also, I was not able to run firefox pdf viewer that was build based on the https://bugzilla.mozilla.org/show_bug.cgi?id=942707 patches (just to verify if find works there). Looks like it is not hooked properly.

jmathies · 2014-08-07T13:05:28Z

Sure, I’m on pto till Monday though, will look at it when I’m back.

Jim

From: Yury Delendik [mailto:[email protected]]
Sent: Wednesday, August 06, 2014 6:15 PM
To: mozilla/pdf.js
Cc: Jim Mathies
Subject: Re: [pdf.js] Firefox pdf viewer support for e10s (Bug 942707). (#5115)

@jmathies https://github.com/jmathies I was able to run it as an extension, however find functionality does not work. Can you look you that.

Also, I was not able to run firefox pdf viewer that was build based on the https://bugzilla.mozilla.org/show_bug.cgi?id=942707 patches (just to verify if find works there). Looks like it is not hooked properly.

—
Reply to this email directly or view it on GitHub #5115 (comment) .Image removed by sender.

yurydelendik · 2014-09-06T00:57:52Z

updated with new set of patches from @jmathies -- looks like extension works now for e10s

/botio-windows preview

pdfjsbot · 2014-09-06T00:57:53Z

From: Bot.io (Windows)

Received

Command cmd_preview from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/cf58ca7c5a891ed/output.txt

pdfjsbot · 2014-09-06T00:58:46Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/cf58ca7c5a891ed/output.txt

Total script time: 0.87 mins

Published

Viewer: http://107.22.172.223:8877/cf58ca7c5a891ed/web/viewer.html
B2G Viewer: http://107.22.172.223:8877/cf58ca7c5a891ed/extensions/b2g/content/web/viewer.html
Extension: http://107.22.172.223:8877/cf58ca7c5a891ed/extensions/firefox/pdf.js.xpi
Extension (AMO): http://107.22.172.223:8877/cf58ca7c5a891ed/extensions/firefox/pdf.js.amo.xpi

yurydelendik · 2014-09-08T20:46:43Z

/botio-windows preview

pdfjsbot · 2014-09-08T20:46:44Z

From: Bot.io (Windows)

Received

Command cmd_preview from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/3d9639a941d1cca/output.txt

pdfjsbot · 2014-09-08T20:47:42Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/3d9639a941d1cca/output.txt

Total script time: 0.96 mins

Published

Viewer: http://107.22.172.223:8877/3d9639a941d1cca/web/viewer.html
B2G Viewer: http://107.22.172.223:8877/3d9639a941d1cca/extensions/b2g/content/web/viewer.html
Extension: http://107.22.172.223:8877/3d9639a941d1cca/extensions/firefox/pdf.js.xpi
Extension (AMO): http://107.22.172.223:8877/3d9639a941d1cca/extensions/firefox/pdf.js.amo.xpi

Snuffleupagus · 2014-09-08T20:49:26Z

extensions/firefox/content/PdfjsContentUtils.jsm

+   * note, the pref names here are cross-checked against a list of
+   * approved pdfjs prefs in chrome utils. If you add additional pdfjs
+   * prefs update the defaults in DEFAULT_PREFERENCES and in chrome
+   * utils _allowedPrefNames.


The last sentence is no longer applicable.

jmathies · 2014-09-09T14:12:29Z

Full output at http://107.22.172.223:8877/3d9639a941d1cca/output.txt

Total script time: 0.96 mins
Published

Viewer: http://107.22.172.223:8877/3d9639a941d1cca/web/viewer.html
B2G Viewer: http://107.22.172.223:8877/3d9639a941d1cca/extensions/b2g/content/web/viewer.html
Extension: http://107.22.172.223:8877/3d9639a941d1cca/extensions/firefox/pdf.js.xpi
Extension (AMO): http://107.22.172.223:8877/3d9639a941d1cca/extensions/firefox/pdf.js.amo.xpi

This extension is working for me. tested basic functionality, everything seems ok.

yurydelendik · 2014-09-09T14:22:57Z

This extension is working for me. tested basic functionality, everything seems ok.

Thank you for testing. One question is left: does my code properly cleans itself out when the extension is disabled/restarted/reinstalled/removed?

Snuffleupagus · 2014-09-09T14:27:17Z

I think you need to add a line like:

cleanupJSSource(MOZCENTRAL_CONTENT_DIR + '/PdfjsChromeUtils.jsm');

for node make firefox/mozcentral in https://github.com/mozilla/pdf.js/blob/master/make.js to avoid a file header in the middle of the code.

jmathies · 2014-09-09T17:03:18Z

I tested install/uninstall. Uninstall required a restart to get the extension unloaded, but once I reboot the built-in rev came back.

From: Yury Delendik [mailto:[email protected]]
Sent: Tuesday, September 09, 2014 9:23 AM
To: mozilla/pdf.js
Cc: Jim Mathies
Subject: Re: [pdf.js] Firefox pdf viewer support for e10s (Bug 942707). (#5115)

This extension is working for me. tested basic functionality, everything seems ok.

Thank you for testing. One question is left: does my code properly cleans itself out when the extension is disabled/restarted/reinstalled/removed?

—
Reply to this email directly or view it on GitHub #5115 (comment) .Image removed by sender.

brendandahl · 2014-09-16T18:04:11Z

extensions/firefox/bootstrap.js

  // Load the component and register it.
-  pdfStreamConverterUrl = aData.resourceURI.spec +
-                          'content/PdfStreamConverter.jsm';
+  var pdfStreamConverterUrl = pdfBaseUrl + 'content/PdfStreamConverter.jsm';


Why are we registering the stream converter here and in content.js for e10s?

That's how I understand: content for e10s content window, and browser for non-e10s window.

See browser/extensions/pdfjs/content/pdfjschildbootstrap.js

brendandahl · 2014-09-16T20:21:31Z

extensions/firefox/bootstrap.js

+  if (e10sEnabled) {
+    let globalMM = Cc['@mozilla.org/globalmessagemanager;1']
+      .getService(Ci.nsIMessageListenerManager);
+    globalMM.sendSyncMessage('PDFJS:Child:shutdown');


Disabling the addon:
Exception running bootstrap method shutdown on [email protected]: TypeError: globalMM.sendSyncMessage is not a function (resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/bdahl/Library/Application%20Support/Firefox/Profiles/dx3hp1j0.e10s/extensions/[email protected]!/bootstrap.js:182:4) JS Stack trace: shutdown@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/bdahl/Library/Application%20Support/Firefox/Profiles/dx3hp1j0.e10s/extensions/[email protected]:182:5 < [email protected]:4304:9 < [email protected]:4418:1 < [email protected]:6741:9 < [email protected]:1084:11 < oncommand@about:addons:1:1

brendandahl · 2014-09-16T20:23:01Z

extensions/firefox/bootstrap.js

  // Load the component and register it.
-  pdfStreamConverterUrl = aData.resourceURI.spec +
-                          'content/PdfStreamConverter.jsm';
+  var pdfStreamConverterUrl = pdfBaseUrl + 'content/PdfStreamConverter.jsm';
  Cu.import(pdfStreamConverterUrl);
  pdfStreamConverterFactory.register(PdfStreamConverter);


During disable:
WARN Exception running bootstrap method startup on [email protected]: [Exception... "Component returned failure code: 0xc1f30100 (NS_ERROR_FACTORY_EXISTS) [nsIComponentRegistrar.registerFactory]" nsresult: "0xc1f30100 (NS_ERROR_FACTORY_EXISTS)" location: "JS frame :: resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/bdahl/Library/Application%20Support/Firefox/Profiles/dx3hp1j0.e10s/extensions/[email protected]!/bootstrap.js :: register :: line 99" data: no] Stack trace: register()@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/bdahl/Library/Application%20Support/Firefox/Profiles/dx3hp1j0.e10s/extensions/[email protected]!/bootstrap.js:99 < startup()@resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///Users/bdahl/Library/Application%20Support/Firefox/Profiles/dx3hp1j0.e10s/extensions/[email protected]!/bootstrap.js:152 < XPI_callBootstrapMethod()@resource://gre/modules/addons/XPIProvider.jsm:4304 < XPI_updateAddonDisabledState()@resource://gre/modules/addons/XPIProvider.jsm:4427 < AddonWrapper_userDisabledSetter()@resource://gre/modules/addons/XPIProvider.jsm:6741 < set_userDisabled()@extensions.xml:1084 < oncommand()@about:addons:1 < file:unknown

yurydelendik · 2014-09-16T23:17:01Z

/botio-windows preview

pdfjsbot · 2014-09-16T23:17:02Z

From: Bot.io (Windows)

Received

Command cmd_preview from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/56f027959876b4b/output.txt

pdfjsbot · 2014-09-16T23:18:07Z

From: Bot.io (Windows)

Success

Full output at http://107.22.172.223:8877/56f027959876b4b/output.txt

Total script time: 1.07 mins

Published

Viewer: http://107.22.172.223:8877/56f027959876b4b/web/viewer.html
B2G Viewer: http://107.22.172.223:8877/56f027959876b4b/extensions/b2g/content/web/viewer.html
Extension: http://107.22.172.223:8877/56f027959876b4b/extensions/firefox/pdf.js.xpi
Extension (AMO): http://107.22.172.223:8877/56f027959876b4b/extensions/firefox/pdf.js.amo.xpi

Firefox pdf viewer support for e10s (Bug 942707).

timvandermeij added this to the 2014 Q3 milestone Aug 1, 2014

Snuffleupagus reviewed Aug 5, 2014
View reviewed changes

timvandermeij added 1-core labels Aug 11, 2014

Firefox pdf viewer support for e10s (Bug 942707).

f2096fe

yurydelendik force-pushed the e10s branch from 6168649 to 9df6fca Compare September 6, 2014 00:56

yurydelendik force-pushed the e10s branch 2 times, most recently from dceadf0 to aaafcc6 Compare September 8, 2014 20:29

Lint error fix for pdfjschildbootstrap.js

782ed5c

yurydelendik force-pushed the e10s branch 2 times, most recently from d71d796 to 0199d16 Compare September 8, 2014 20:45

Snuffleupagus reviewed Sep 8, 2014
View reviewed changes

Adds DEFAULT_PREFERENCES to the PdfjsChromeUtils

7ae7fd3

yurydelendik force-pushed the e10s branch from 0199d16 to 58f8074 Compare September 8, 2014 20:53

brendandahl reviewed Sep 16, 2014
View reviewed changes

timvandermeij mentioned this pull request Sep 16, 2014

PDF not loaded in Nightly #5299

Closed

yurydelendik force-pushed the e10s branch from 58f8074 to 154bc6c Compare September 16, 2014 19:24

brendandahl reviewed Sep 16, 2014
View reviewed changes

yurydelendik force-pushed the e10s branch 2 times, most recently from 0257c7c to 7faa801 Compare September 16, 2014 22:59

Loading PDF.js extension into e10s windows

1cda4c7

yurydelendik force-pushed the e10s branch from 7faa801 to 1cda4c7 Compare September 17, 2014 21:14

brendandahl added a commit that referenced this pull request Sep 17, 2014

Merge pull request #5115 from yurydelendik/e10s

f6745d5

Firefox pdf viewer support for e10s (Bug 942707).

brendandahl merged commit f6745d5 into mozilla:master Sep 17, 2014

Snuffleupagus mentioned this pull request Sep 18, 2014

For |make firefox/mozcentral| builds, add cleanupJSSource to remove duplicate file headers in extensions/firefox/content/PdfjsChromeUtils.jsm #5312

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Firefox pdf viewer support for e10s (Bug 942707). #5115

Firefox pdf viewer support for e10s (Bug 942707). #5115

yurydelendik commented Aug 1, 2014

Snuffleupagus Aug 5, 2014

yurydelendik commented Aug 6, 2014

jmathies commented Aug 7, 2014

yurydelendik commented Sep 6, 2014

pdfjsbot commented Sep 6, 2014

pdfjsbot commented Sep 6, 2014

yurydelendik commented Sep 8, 2014

pdfjsbot commented Sep 8, 2014

pdfjsbot commented Sep 8, 2014

Snuffleupagus Sep 8, 2014

jmathies commented Sep 9, 2014

yurydelendik commented Sep 9, 2014

Snuffleupagus commented Sep 9, 2014

jmathies commented Sep 9, 2014

brendandahl Sep 16, 2014

yurydelendik Sep 16, 2014

brendandahl Sep 16, 2014

brendandahl Sep 16, 2014

yurydelendik commented Sep 16, 2014

pdfjsbot commented Sep 16, 2014

pdfjsbot commented Sep 16, 2014

Firefox pdf viewer support for e10s (Bug 942707). #5115

Firefox pdf viewer support for e10s (Bug 942707). #5115

Conversation

yurydelendik commented Aug 1, 2014

Snuffleupagus Aug 5, 2014

Choose a reason for hiding this comment

yurydelendik commented Aug 6, 2014

jmathies commented Aug 7, 2014

yurydelendik commented Sep 6, 2014

pdfjsbot commented Sep 6, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Sep 6, 2014

From: Bot.io (Windows)

Success

Published

yurydelendik commented Sep 8, 2014

pdfjsbot commented Sep 8, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Sep 8, 2014

From: Bot.io (Windows)

Success

Published

Snuffleupagus Sep 8, 2014

Choose a reason for hiding this comment

jmathies commented Sep 9, 2014

yurydelendik commented Sep 9, 2014

Snuffleupagus commented Sep 9, 2014

jmathies commented Sep 9, 2014

brendandahl Sep 16, 2014

Choose a reason for hiding this comment

yurydelendik Sep 16, 2014

Choose a reason for hiding this comment

brendandahl Sep 16, 2014

Choose a reason for hiding this comment

brendandahl Sep 16, 2014

Choose a reason for hiding this comment

yurydelendik commented Sep 16, 2014

pdfjsbot commented Sep 16, 2014

From: Bot.io (Windows)

Received

pdfjsbot commented Sep 16, 2014

From: Bot.io (Windows)

Success

Published