Closed
Bug 1161169
Opened 9 years ago
Closed 8 years ago
[e10s] mContentParent pointer in plugin code is worrisome
Categories
(Core Graveyard :: Plug-ins, defect, P2)
Core Graveyard
Plug-ins
Tracking
(e10s+, firefox40 affected, firefox47 fixed)
RESOLVED
FIXED
mozilla47
People
(Reporter: billm, Assigned: bugzilla)
Details
Attachments
(1 file)
I was looking at this pointer today and a couple things worry me. First, it isn't going to work when we have multiple content processes, since they can share the same plugin. Second, there doesn't appear to be any code to clear mContentParent if the ContentParent goes away. That's probably not very likely, but it would still be better to get rid of this pointer.
Updated•9 years ago
|
tracking-e10s:
--- → +
Updated•8 years ago
|
Priority: -- → P2
Assignee | ||
Comment 2•8 years ago
|
||
mContentParent is really just to be used while handling a synchronous ContentParent::RecvLoadPlugin call when async plugin init turned on. In any other context, using it will be unsafe. This patch adds comments and assertions to ensure that this value isn't set otherwise, and converts the one use of mContentParent outside of async plugin init to use an alternative mechanism for identifying the content process. Review commit: https://reviewboard.mozilla.org/r/37913/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/37913/
Attachment #8726294 -
Flags: review?(wmccloskey)
Attachment #8726294 -
Flags: review?(jmathies)
Reporter | ||
Comment 3•8 years ago
|
||
Comment on attachment 8726294 [details] MozReview Request: Bug 1161169: Clean up usage of mContentParent and clearly identify it as specifically for async plugin init; r?billm, jimm https://reviewboard.mozilla.org/r/37913/#review34533 Thanks Aaron!
Attachment #8726294 -
Flags: review?(wmccloskey) → review+
Updated•8 years ago
|
Attachment #8726294 -
Flags: review?(jmathies) → review+
Comment 4•8 years ago
|
||
Comment on attachment 8726294 [details] MozReview Request: Bug 1161169: Clean up usage of mContentParent and clearly identify it as specifically for async plugin init; r?billm, jimm https://reviewboard.mozilla.org/r/37913/#review34669 ::: dom/plugins/ipc/PluginModuleParent.h:425 (Diff revision 1) > + base::ProcessId aContentPid, Please add an entry for this parameter up in the comment header.
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8726294 [details] MozReview Request: Bug 1161169: Clean up usage of mContentParent and clearly identify it as specifically for async plugin init; r?billm, jimm Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37913/diff/1-2/
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3438e0228ed7
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•