1189887 - Fix dom.ipc.plugins.asyncInit on e10s-enabled channels

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, defect, P5)

Description

[(No longer employed by Mozilla) Aaron Klotz]

This pref should be enabled by default when e10s is deactivated (at least until the e10s-specific asyncInit bugs are ironed out). Right now it is disabled by default even when e10s is disabled.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Patch

Whoops, missed an #include

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Telemetry Patch

To make sure that telemetry reflects reality. I'll revert this when the e10s bugs in asyncInit are resolved.

Comment 4

[Jim Mathies [:jimm]]

Comment on attachment 8641856 [details] [diff] [review]
Patch

Having that pref not take effect is a bit misleading but it's an internal thing I guess, seems ok.

Comment 5

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8641861 [details] [diff] [review]
Telemetry Patch

Review of attachment 8641861 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/TelemetrySession.jsm
@@ +1101,5 @@
>      let ret = {
>        reason: reason,
>        revision: HISTOGRAMS_FILE_VERSION,
> +      asyncPluginInit: (Preferences.get(PREF_ASYNC_PLUGIN_INIT, false) &&
> +                        !Services.appinfo.browserTabsRemoteAutostart),

I think we should just fix bug 1179189 instead, making settings.e10sEnabled use |appinfo.browserTabsRemoteAutostart| - then we don't need to change anything about asyncPluginInit.

Comment 6

[Georg Fritzsche [:gfritzsche]]

Currently you can already filter this out in analysis using the E10S_AUTOSTART histogram.

Comment 7

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Georg Fritzsche [:gfritzsche] from comment #5)
> Comment on attachment 8641861 [details] [diff] [review]
> Telemetry Patch
> 
> Review of attachment 8641861 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/telemetry/TelemetrySession.jsm
> @@ +1101,5 @@
> >      let ret = {
> >        reason: reason,
> >        revision: HISTOGRAMS_FILE_VERSION,
> > +      asyncPluginInit: (Preferences.get(PREF_ASYNC_PLUGIN_INIT, false) &&
> > +                        !Services.appinfo.browserTabsRemoteAutostart),
> 
> I think we should just fix bug 1179189 instead, making settings.e10sEnabled
> use |appinfo.browserTabsRemoteAutostart| - then we don't need to change
> anything about asyncPluginInit.

It's not clear to me how that bug applies to this one: the other patch in this bug modifies asyncInit to be disabled when e10s is enabled (temporarily, until some remaining issues are ironed out), even when the pref is set to true. The point of this patch is to ensure that the state of the flag in telemetry is synchronized with the effective state of the feature in the plugin code.

Comment 8

[Georg Fritzsche [:gfritzsche]]

I am saying that you can already pull this data by looking at both asyncPluginInit & E10S_AUTOSTART (or now e10sEnabled), so i'm not sure why we need to duplicate this logic temporarily.

Alternatively we could add, say, nsIPluginHost.asyncPluginInit and use that in both places.

Comment 9

[Pulsebot]

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

Comment 10

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Georg Fritzsche [:gfritzsche] from comment #8)
> I am saying that you can already pull this data by looking at both
> asyncPluginInit & E10S_AUTOSTART (or now e10sEnabled), so i'm not sure why
> we need to duplicate this logic temporarily.
> 
> Alternatively we could add, say, nsIPluginHost.asyncPluginInit and use that
> in both places.

I don't really feel like adding a bunch of nsPluginHost goop to accommodate an ephemeral change. I guess we'll stick with pulling e10sEnabled in analyses for now.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

backed out for mochitest-3 failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/26cc89d943d6

https://treeherder.mozilla.org/logviewer.html#?job_id=12534509&repo=mozilla-inbound

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

I've added a dependency on bug 1172627; look like I need to fix that first.

Comment 13

[(No longer employed by Mozilla) Aaron Klotz]

async init is on hold. Setting this to WONTFIX. If we resurrect it, we will need to bring this back.