Closed Bug 1130032 Opened 5 years ago Closed 5 years ago
Missing plugin UI is broken
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.
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.
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.
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?
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.
A debug local build doesn't reproduce this issue.
A release local build doesn't reproduce either.
Maybe a regression window will help.
Nightly-debug linux doesn't reproduce either.
2014-10-01-03-02-05 nightly doesn't reproduce, so that's at least an early-bound on the bisection.
Nightly range 17-oct to 18-oct: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ae1dfa192faf&tochange=92c87e95915e
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
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....
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!
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.
(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.
Flags: needinfo?(jdemooij) → needinfo?(ydelendik)
It is safe to add something like: if (state === null) state = Ci.nsIObjectLoadingContent.PLUGIN_UNSUPPORTED; Eventually we will remove flash telemetry code.
Attachment #8560695 - Flags: review?(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.
Attachment #8560695 - Flags: review?(mconley) → review+
How about: >+let state = this._getPluginInfo(plugin).fallbackType || Ci.nsIObjectLoadingContent.PLUGIN_UNSUPPORTED;
Yury, can you fill the uplift request for aurora & beta? Thanks
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
Verified as fixed using the following environment: FF 36.0b9 Build Id:20150212154903 OS: Win 8.1 x86
Verified as fixed on Win 8.1 64-bit using Firefox 37 beta 1 and Nightly 38.0a1.
You need to log in before you can comment on or make changes to this bug.