All users were logged out of Bugzilla on October 13th, 2018

[jsplugins] Add a way to unregister the plugin in mortar-pdf's bootstrap.js

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ywu, Assigned: ywu)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

2 years ago
We register our plugin every time when we install the add-on but haven't had a way to unregister it. This bug is going to put a way to unregister the plugin in mortar-pdf's bootstrap.js
(Assignee)

Comment 1

2 years ago
Created attachment 8839022 [details] [diff] [review]
Add a way to unregister the plugin in mortarpdf bootstrap

We register our plugin in mortar add-on when we install it but we haven't done the unregistration when the application is shutting down. This patch is adding unregistration stuff into shutdown.
(Assignee)

Updated

2 years ago
Attachment #8839022 - Flags: review?(peterv)
(Assignee)

Comment 2

2 years ago
Created attachment 8842342 [details] [diff] [review]
Bug1340472-Add-a-way-to-unregister-the-plugin-in-mor.patch

This patch is basically the same as previous one except that I change some naming to more fit to our preference setting. btw our team decided to put the preference in Bug 1338476 to "pdfium.disabled".
Attachment #8839022 - Attachment is obsolete: true
Attachment #8839022 - Flags: review?(peterv)
(Assignee)

Comment 3

2 years ago
Comment on attachment 8842342 [details] [diff] [review]
Bug1340472-Add-a-way-to-unregister-the-plugin-in-mor.patch

hey Evelyn,

could you help with this patch? I add mime type unregistration into our addon. We should do unregistration when we shutdown the addon.

thanks!
Attachment #8842342 - Flags: review?(ehung)

Comment 4

2 years ago
Comment on attachment 8842342 [details] [diff] [review]
Bug1340472-Add-a-way-to-unregister-the-plugin-in-mor.patch

Review of attachment 8842342 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the patch. r+ with my comment addressed.

::: browser/extensions/mortar/host/pdf/bootstrap.js
@@ +11,5 @@
>    dump("sandboxScript " + sandbox.pluginElement + "\n");
>    Components.classes["@mozilla.org/moz/jssubscript-loader;1"].getService(Components.interfaces.mozIJSSubScriptLoader).loadSubScript("resource://ppapipdf.js/ppapi-content-sandbox.js", sandbox);
>  }
>  
> +let bootstrapData;

Please avoid using global variable. Can we do pdfium.init(data)?

@@ +12,5 @@
>    Components.classes["@mozilla.org/moz/jssubscript-loader;1"].getService(Components.interfaces.mozIJSSubScriptLoader).loadSubScript("resource://ppapipdf.js/ppapi-content-sandbox.js", sandbox);
>  }
>  
> +let bootstrapData;
> +let handlerURI = "chrome://ppapipdf.js/content/viewer.html";

This should be a const.

@@ +58,3 @@
>  
> +    let rng = Cc["@mozilla.org/security/random-generator;1"].createInstance(Ci.nsIRandomGenerator);
> +    Services.ppmm.addMessageListener("ppapi.js:generateRandomBytes", ({ bootstrapData }) => {

We don't need to rename here, because the `data` is an argument from message sender, not the `data` variable in this JS context.

@@ +62,5 @@
> +    });
> +  },
> +  uninit() {
> +    let pluginHost = Cc["@mozilla.org/plugin/host;1"].getService(Ci.nsIPluginHost);
> +    pluginHost.unregisterFakePlugin(handlerURI);

We also need to remove "ppapi.js:generateRandomBytes" message listener that registered in init().
Attachment #8842342 - Flags: review?(ehung) → review+
(Assignee)

Comment 5

2 years ago
Created attachment 8843839 [details] [diff] [review]
0001-Bug1340472-Add-a-way-to-unregister-the-plugin-in-mor.patch

Updated the patch by addressing reviewer's comment.

Result of try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2bb573f970899c4d9406b703c750d2e16b2dcc0f
Attachment #8842342 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 6

2 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c112918872bd
Add a way to unregister the plugin in mortar-pdf's bootstrap. r=ehung.
Keywords: checkin-needed

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c112918872bd
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.