Closed Bug 1091621 Opened 7 years ago Closed 7 years ago

Plugins can't load w/e10s (bug 641685 regression)

Categories

(Core :: Plug-ins, defect)

x86_64
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
e10s + ---

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(1 file)

Attached patch fixSplinter 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 on attachment 8514281 [details] [diff] [review]
fix

Why isn't the epoch ending up as "1" after the initial load in the chrome process?
Flags: needinfo?(jmathies)
(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)
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)
Yes, that's fine.
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: nobody → jmathies
tracking-e10s: --- → +
https://hg.mozilla.org/mozilla-central/rev/b05655e457ad
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Is this going to make the x64-PGO inbound?  I'm testing with these and have Flash problems that this patch seems to fix.
You need to log in before you can comment on or make changes to this bug.