1130032 - Missing plugin UI is broken

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

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

In Windows 8, using Nightly (non-e10s), Aurora, and beta:

1. Uninstall Flash
2. Visit http://www.homestarrunner.com/

AR:
See a plain gray box with no next

ER:
See "A plugin is needed to display this content."

This works correctly in release.

Comment 1

[Benjamin Smedberg]

This artificial testcase WFM http://benjamin.smedbergs.us/tests/ctptests/missing-solo.html

But I can reproduce this on homestarrunner.com

I tried to debug this at http://hg.mozilla.org/mozilla-central/annotate/58ce6051edf5/browser/modules/PluginContent.jsm#l394 using the builtin browser toolbox. Oddly, we seem to end up skipping over hunks of code for no reason when I step through:

Stepping, I get to
http://hg.mozilla.org/mozilla-central/annotate/58ce6051edf5/browser/modules/PluginContent.jsm#l394
and then the debugger immediately skips to
http://hg.mozilla.org/mozilla-central/annotate/58ce6051edf5/browser/modules/PluginContent.jsm#l421

In any case, that's the place to keep poking at this.

Comment 2

[Benjamin Smedberg]

After talking with jimb a bit, I'm going to try this in a self build to see if the symptoms still show up; but we think this might actually be some kind of JS execution bug. We almost ruled out debugger sourcemap errors because setting interior breakpoints don't hit anything.

Comment 3

[Lawrence Mandel [:lmandel] (use needinfo)]

I'm tracking 36+ in order to keep this on the radar while we figure out what's going on.

Benjamin - Looks like you're working on this. Can we assign this one to you?

Comment 4

[Benjamin Smedberg]

I'm not going to commit to this, no. In the grand scheme of things, I'd probably ship with this regression except for the really weird bit about this looking like a JS execution/codegen bug.

Comment 5

[Benjamin Smedberg]

A debug local build doesn't reproduce this issue.

Comment 6

[Benjamin Smedberg]

A release local build doesn't reproduce either.

Comment 7

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

Maybe a regression window will help.

Comment 8

[Benjamin Smedberg]

Nightly-debug linux doesn't reproduce either.

Comment 9

[Benjamin Smedberg]

2014-10-01-03-02-05 nightly doesn't reproduce, so that's at least an early-bound on the bisection.

Comment 10

[Benjamin Smedberg]

Nightly range 17-oct to 18-oct: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ae1dfa192faf&tochange=92c87e95915e

Comment 11

[Benjamin Smedberg]

Here are the JS changes and other possible suspects from the regression range:

    8c7d2cd4fa06    Tom Schuster — Bug 1081660 - Remove property iterator from JSAPI. r=Waldo
    9702f1f2f133    Jason Orendorff — Bug 1083204 - Handlify js::GetPropertyKeys and Snapshot. r=efaust.
    a2458d9afc81    Jason Orendorff — Bug 1081280 - Rename BaseProxyHandler::keys -> getOwnEnumerablePropertyKeys. r=efaust.
    075cf4911854    Jason Orendorff — Bug 1081255 - Rewrite comments in jsproxy.h; reclassify the methods a bit. No change in behavior. r=efaust, r=bz, r=jwalden.
    9a7fd8fd00b3    Jason Orendorff — Bug 1082672, part 4 - Change XrayWrapper code to be able to resolve symbol-keyed methods. r=bz, r=bholley.
    f2214b9e3333    Jason Orendorff — Bug 1082672, part 3 - Add some more symbol support for DOM bindings. r=bz.
    bff9837442af    Jason Orendorff — Bug 1082672, part 2 - Change mozilla::dom::GetArrayIndexFromId to cope with symbols. r=bz.
    ac0d55e594f1    Jason Orendorff — Bug 1082672, part 1 - Add JSAPI macros JS_SYM_FN etc. to support defining functions with well-known symbol keys. r=Waldo.
    c0d3ea180fe0    Jason Orendorff — Add some test cases involving Symbol.iterator as a prelude to bug 918828. no_r=me, testonly.
    a758ca998666    Mike Hommey — Bug 609976 - Fold mozjs.dll back into xul.dll. r=ehsan
    b56d94c7261a    Jan de Mooij — Bug 987560 - Greatly refactor generator implementation. Patch mostly written by Andy Wingo. r=wingo

Comment 12

[Matthias Versen [:Matti]]

 3:44.86 LOG: MainThread Bisector INFO Last good revision: ae1dfa192faf
 3:44.86 LOG: MainThread Bisector INFO First bad revision: 92c87e95915e
 3:44.86 LOG: MainThread Bisector INFO Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ae1dfa192faf&tochange=92c87e95915e

20:10.04 LOG: MainThread Bisector INFO Last good revision: 9a7fd8fd00b3
20:10.04 LOG: MainThread Bisector INFO First bad revision: dc2d310c063c
20:10.04 LOG: MainThread Bisector INFO Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9a7fd8fd00b3&tochange=dc2d310c063c

I can not reproduce the issue in my local non-release build....

Comment 13

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

Ah! It looks like local builds will return false for Services.telemetry.canSend:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/Telemetry.cpp#2910
That's probably why we can't reproduce this in self-made builds.

Thanks Matthias!

Comment 14

[Jim Blandy :jimb]

If it really is a JS execution problem, then I would be suspicious of bug 987560, the generator changes, in that list. That was a very large change, and it touches code generation.

Comment 15

[Benjamin Smedberg]

Jandem asked whether disabling javascript.options.baselinejit or javascript.options.ion fixed the problem, and local testing indicates that it makes no difference.

Comment 16

[Jan de Mooij [:jandem]]

(In reply to Bill McCloskey (:billm) from comment #13)
> Ah! It looks like local builds will return false for
> Services.telemetry.canSend:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/
> Telemetry.cpp#2910
> That's probably why we can't reproduce this in self-made builds.

Yes, it repros locally if I make that return true. The problem is in _recordFlashPluginTelemetry:

      let state = this._getPluginInfo(plugin).fallbackType;
      Services.telemetry.getHistogramById('FLASH_PLUGIN_STATES')
                       .add(state);

state is null, so add(state) throws "Not a number". Commenting out these lines "fixes" it.

Comment 17

[Yury Delendik (:yury)]

It is safe to add something like:

if (state === null) state = Ci.nsIObjectLoadingContent.PLUGIN_UNSUPPORTED;

Eventually we will remove flash telemetry code.

Comment 18

[Yury Delendik (:yury)]

Attachment: Guard against state==null

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fa7b223b2206

Comment 19

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8560695 [details] [diff] [review]
Guard against state==null

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

By inspection, this looks good - though I'm not currently in a position to test this. We might want QA to bang the tires on this just to be sure.

Comment 20

[Carsten Book [:Tomcat]]

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

Comment 21

[Philip Chee]

How about:
>+let state = this._getPluginInfo(plugin).fallbackType || Ci.nsIObjectLoadingContent.PLUGIN_UNSUPPORTED;

Comment 22

[Ryan VanderMeulen [:RyanVM]]

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

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Yury, can you fill the uplift request for aurora & beta? Thanks

Comment 24

[Yury Delendik (:yury)]

Comment on attachment 8560695 [details] [diff] [review]
Guard against state==null

Approval Request Comment
[Feature/regressing bug #]: Bug 1075088
[User impact if declined]: Missing plugin UI 
[Describe test coverage new/current, TreeHerder]: in m-c, https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8831ddb0041, https://treeherder.mozilla.org/#/jobs?repo=try&revision=48881bbf0f06
[Risks and why]: Very low -- trivial patch
[String/UUID change made/needed]: none

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/10d1df99ba8e

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5cfeb5068e2b

Comment 27

[Catalin Varga [QA][:VarCat]]

Verified as fixed using the following environment:

FF 36.0b9
Build Id:20150212154903
OS: Win 8.1 x86

Comment 28

[Petruta Horea [:phorea], Desktop Test Engineering]

Verified as fixed on Win 8.1 64-bit using Firefox 37 beta 1 and Nightly 38.0a1.