Add a run ID to NPAPI plugins.

RESOLVED FIXED in Firefox 40

Status

()

Core
Plug-ins
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

unspecified
mozilla40
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10sm6+, firefox40 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments, 1 obsolete attachment)

Split out from bug 1110887.

In order to differentiate different "runs" of a plugin process, I propose adding a run ID that can be exposed via nsIObjectLoadingContent. This is necessary for the plugin crash UI work I'm doing in bug 1110887.

While the run ID seems to serve the same purpose as the process ID, I felt like the process ID is too much of a system-level bit of data to expose to chrome-level JS.
Created attachment 8583955 [details]
MozReview Request: bz://1148012/mconley

/r/6163 - Bug 1148012 - Add a run ID for plugins to differentiate subsequent runs of the same plugins. r=?
/r/6165 - Bug 1148012 - Expose run ID through nsIObjectLoadingContent.idl. r=?
/r/6167 - Bug 1148012 - Send the run ID and plugin name along with the plugin-crashed observer notification. r=?

Pull down these commits:

hg pull review -r 2c405f53cc691446b6dfbae1fb7ea194ba18e674
Attachment #8583955 - Flags: review?(jocheng)
Attachment #8583955 - Flags: review?(jmathies)
Comment on attachment 8583955 [details]
MozReview Request: bz://1148012/mconley

/r/6163 - Bug 1148012 - Add a run ID for plugins to differentiate subsequent runs of the same plugins. r=?
/r/6165 - Bug 1148012 - Expose run ID through nsIObjectLoadingContent.idl. r=?
/r/6167 - Bug 1148012 - Send the run ID and plugin name along with the plugin-crashed observer notification. r=?

Pull down these commits:

hg pull review -r 2c405f53cc691446b6dfbae1fb7ea194ba18e674
Comment on attachment 8583955 [details]
MozReview Request: bz://1148012/mconley

/r/6163 - Bug 1148012 - Add a run ID for plugins to differentiate subsequent runs of the same plugins. r=?
/r/6165 - Bug 1148012 - Expose run ID through nsIObjectLoadingContent.idl. r=?
/r/6167 - Bug 1148012 - Send the run ID and plugin name along with the plugin-crashed observer notification. r=?

Pull down these commits:

hg pull review -r 2c405f53cc691446b6dfbae1fb7ea194ba18e674
Attachment #8583955 - Flags: review?(jocheng) → review?(joshmoz)
Hey Josh - additional context for this is in bug 1110887 comment 10 and bug 1110887 comment 12.
https://reviewboard.mozilla.org/r/6165/#review5163

::: dom/base/nsObjectLoadingContent.cpp
(Diff revision 1)
> +  if (NS_WARN_IF(!aRunID) {

Bah - goofed here in a refactor. Missing a closing parenthesis - I'll update the patch...
Comment on attachment 8583955 [details]
MozReview Request: bz://1148012/mconley

/r/6163 - Bug 1148012 - Add a run ID for plugins to differentiate subsequent runs of the same plugins. r=?
/r/6165 - Bug 1148012 - Expose run ID through nsIObjectLoadingContent.idl. r=?
/r/6167 - Bug 1148012 - Send the run ID and plugin name along with the plugin-crashed observer notification. r=?

Pull down these commits:

hg pull review -r 05cffc803255f549feeee8aa96a6df6ade4247ff

Comment 7

3 years ago
Comment on attachment 8583955 [details]
MozReview Request: bz://1148012/mconley

https://reviewboard.mozilla.org/r/6161/#review5265

::: dom/plugins/base/nsNPAPIPluginInstance.cpp
(Diff revision 2)
> +    return NS_ERROR_FAILURE;

Always use {} for if blocks in this code.

::: dom/plugins/base/nsNPAPIPluginInstance.cpp
(Diff revision 2)
> +  PluginLibrary* library = mPlugin->GetLibrary();

Probably best to check that mPlugin is not null.
Attachment #8583955 - Flags: review?(joshmoz)
Comment on attachment 8583955 [details]
MozReview Request: bz://1148012/mconley

/r/6163 - Bug 1148012 - Add a run ID for plugins to differentiate subsequent runs of the same plugins. r=jimm.
/r/6165 - Bug 1148012 - Expose run ID through nsIObjectLoadingContent.idl. r=?
/r/6167 - Bug 1148012 - Send the run ID and plugin name along with the plugin-crashed observer notification. r=?

Pull down these commits:

hg pull review -r a19ce82f52e61cd4f9361ee7f9afea5ca1ed6748
Attachment #8583955 - Flags: review?(jmathies) → review?(joshmoz)

Comment 9

3 years ago
Comment on attachment 8583955 [details]
MozReview Request: bz://1148012/mconley

https://reviewboard.mozilla.org/r/6161/#review5499
Attachment #8583955 - Flags: review?(joshmoz) → review+
Comment on attachment 8583955 [details]
MozReview Request: bz://1148012/mconley

/r/6163 - Bug 1148012 - Add a run ID for plugins to differentiate subsequent runs of the same plugins. r=jimm.
/r/6165 - Bug 1148012 - Expose run ID through nsIObjectLoadingContent.idl. r=?
/r/6167 - Bug 1148012 - Send the run ID and plugin name along with the plugin-crashed observer notification. r=?

Pull down these commits:

hg pull review -r a19ce82f52e61cd4f9361ee7f9afea5ca1ed6748
Attachment #8583955 - Flags: review?(mrbkap)
Attachment #8583955 - Flags: review?(joshmoz)
Attachment #8583955 - Flags: review+
I hit this when trying to push to inbound:

remote: ************************** ERROR ****************************
remote:
remote: WebIDL file dom/webidl/HTMLObjectElement.webidl altered in changeset 7ae748601cc4 without DOM peer review
remote:
remote:
remote: Changes to WebIDL files in this repo require review from a DOM peer in the form of r=...
remote: This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding..
remote:
remote: *************************************************************

And I believe mrbkap is a DOM peer.
Comment on attachment 8583955 [details]
MozReview Request: bz://1148012/mconley

r+ for the .webidl change.
Attachment #8583955 - Flags: review+
Comment on attachment 8583955 [details]
MozReview Request: bz://1148012/mconley

/r/6163 - Bug 1148012 - Add a run ID for plugins to differentiate subsequent runs of the same plugins. r=jimm.
/r/6165 - Bug 1148012 - Expose run ID through nsIObjectLoadingContent.idl. r=?
/r/6167 - Bug 1148012 - Send the run ID and plugin name along with the plugin-crashed observer notification. r=?

Pull down these commits:

hg pull review -r a19ce82f52e61cd4f9361ee7f9afea5ca1ed6748
https://hg.mozilla.org/mozilla-central/rev/a423c6fdc3d4
https://hg.mozilla.org/mozilla-central/rev/fb8373ac7558
https://hg.mozilla.org/mozilla-central/rev/84d46805a079
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Comment on attachment 8583955 [details]
MozReview Request: bz://1148012/mconley
Attachment #8583955 - Attachment is obsolete: true
Created attachment 8619883 [details]
MozReview Request: Bug 1148012 - Add a run ID for plugins to differentiate subsequent runs of the same plugins. r=jimm.
Created attachment 8619884 [details]
MozReview Request: Bug 1148012 - Expose run ID through nsIObjectLoadingContent.idl. r=?
Created attachment 8619885 [details]
MozReview Request: Bug 1148012 - Send the run ID and plugin name along with the plugin-crashed observer notification. r=?
You need to log in before you can comment on or make changes to this bug.