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

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Jim Mathies [:jimm]]

Attachment: fix

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.

Comment 1

[Benjamin Smedberg]

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?

Comment 2

[Jim Mathies [:jimm]]

(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

Comment 3

[Benjamin Smedberg]

Right, my question is about why pluginsChanged isn't true in the initial-load case. This seems like an odd special-case.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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?

Comment 5

[Benjamin Smedberg]

Yes, that's fine.

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8514281 [details] [diff] [review]
fix

Could you do that then Jim? Thanks.

Comment 7

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b05655e457ad

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b05655e457ad

Comment 9

[Gary [:streetwolf52]]

Is this going to make the x64-PGO inbound?  I'm testing with these and have Flash problems that this patch seems to fix.