Closed
Bug 1091621
Opened 10 years ago
Closed 10 years ago
Plugins can't load w/e10s (bug 641685 regression)
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(e10s+)
RESOLVED
FIXED
mozilla36
Tracking | Status | |
---|---|---|
e10s | + | --- |
People
(Reporter: jimm, Assigned: jimm)
References
Details
Attachments
(1 file)
(deleted),
patch
|
billm
:
review+
|
Details | Diff | Splinter Review |
This looks like the correct fix - the comparison of the two epoch values in FindPluginsForContent always matches since the initial epoch is always zero, and we don't increment the chrome side value unless we get past that epoc comparison. So the content process never gets a good list of plugins.
Attachment #8514281 -
Flags: review?(wmccloskey)
Comment 1•10 years ago
|
||
Comment on attachment 8514281 [details] [diff] [review]
fix
Why isn't the epoch ending up as "1" after the initial load in the chrome process?
Updated•10 years ago
|
Flags: needinfo?(jmathies)
Assignee | ||
Comment 2•10 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #1)
> Comment on attachment 8514281 [details] [diff] [review]
> fix
>
> Why isn't the epoch ending up as "1" after the initial load in the chrome
> process?
We skip bumping it through the early return in the epoc comparison in the call to nsPluginHost::LoadPlugins()[1]. Maybe the bug is in FindPlugins, where pluginschanged should be set here but isn't? Regardless, my added check/bump when |!ChromeEpoch()| is true solves this.
[1] http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsPluginHost.cpp#2354
Flags: needinfo?(jmathies)
Comment 3•10 years ago
|
||
Right, my question is about why pluginsChanged isn't true in the initial-load case. This seems like an odd special-case.
I think the problem is here:
http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsPluginHost.cpp#1877
When we're reading in plugins for the first time, we use some cached plugin info read out of a text file. If a plugin is already in the cache, we don't consider the plugin list to have changed. This only affects us at startup. And unfortunately it doesn't cause any test failures since the test plugin isn't cached.
I think it might be better to ensure that the epoch is 1 when the chrome process nsPluginHost is created--essentially just move Jim's new code to the constructor. And we should definitely comment this. It's very confusing. Does that sound okay Benjamin?
Flags: needinfo?(benjamin)
Comment on attachment 8514281 [details] [diff] [review]
fix
Could you do that then Jim? Thanks.
Attachment #8514281 -
Flags: review?(wmccloskey) → review+
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → jmathies
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
tracking-e10s:
--- → +
Comment 8•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 9•10 years ago
|
||
Is this going to make the x64-PGO inbound? I'm testing with these and have Flash problems that this patch seems to fix.
Updated•3 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•