Last Comment Bug 935676 - Flash doesn't work on 4.4 KitKat
: Flash doesn't work on 4.4 KitKat
Status: VERIFIED FIXED
: feature
Product: Firefox for Android
Classification: Client Software
Component: Plugins (show other bugs)
: Trunk
: ARM Android
-- normal with 8 votes (vote)
: Firefox 29
Assigned To: James Willcox (:snorp) (jwillcox@mozilla.com)
:
:
Mentors:
Depends on: 956398
Blocks: 957694 1025090 1080342
  Show dependency treegraph
 
Reported: 2013-11-06 11:48 PST by James Willcox (:snorp) (jwillcox@mozilla.com)
Modified: 2016-07-29 14:37 PDT (History)
29 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
+
verified
+
verified
+
verified
verified
verified
verified
26+


Attachments
Hack that should allow flash to load (1.79 KB, patch)
2013-11-14 17:24 PST, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
Hack that should allow flash to load (2.61 KB, patch)
2013-11-15 14:22 PST, Mike Hommey [:glandium]
no flags Details | Diff | Splinter Review
Stub out missing Flash symbols on Android (3.18 KB, patch)
2014-01-03 12:48 PST, James Willcox (:snorp) (jwillcox@mozilla.com)
mh+mozilla: review-
Details | Diff | Splinter Review
Stub out missing Flash symbols (3.25 KB, patch)
2014-01-07 07:33 PST, James Willcox (:snorp) (jwillcox@mozilla.com)
no flags Details | Diff | Splinter Review
Stub out missing Flash symbols (3.03 KB, patch)
2014-01-07 07:35 PST, James Willcox (:snorp) (jwillcox@mozilla.com)
mh+mozilla: feedback-
Details | Diff | Splinter Review
Stub out missing Flash symbols (2.10 KB, patch)
2014-01-08 07:32 PST, James Willcox (:snorp) (jwillcox@mozilla.com)
mh+mozilla: review+
Details | Diff | Splinter Review
Always try to reload invlalid plugins on Android (1.33 KB, patch)
2014-01-08 07:39 PST, James Willcox (:snorp) (jwillcox@mozilla.com)
jaas: review+
Details | Diff | Splinter Review
Stub out missing Flash symbols (1.95 KB, patch)
2014-01-10 07:10 PST, James Willcox (:snorp) (jwillcox@mozilla.com)
snorp: review+
bajaj.bhavana: approval‑mozilla‑aurora+
bajaj.bhavana: approval‑mozilla‑beta+
Details | Diff | Splinter Review
The file is a screen capture (51.33 KB, image/png)
2014-01-21 08:38 PST, Flaviu Cos
no flags Details

Description User image James Willcox (:snorp) (jwillcox@mozilla.com) 2013-11-06 11:48:10 PST
At least on my 2012 Nexus 7 flashed to KitKat, Flash doesn't work and does not appear in about:plugins.
Comment 1 User image Aaron Train [:aaronmt] 2013-11-06 12:16:45 PST
Kevin, does this work on 4.4 with the real bits?
Comment 2 User image Kevin Brosnan [:kbrosnan] 2013-11-06 13:31:48 PST
Confirmed on N5.

Worth a relnote as we are noted for our Flash Player support.
Comment 3 User image Kevin Brosnan [:kbrosnan] 2013-11-06 13:32:51 PST
Jet do you have any Adobe contacts that would know if critical APIs have been removed from Android 4.4 that Flash uses?
Comment 4 User image Aaron Train [:aaronmt] 2013-11-06 14:32:14 PST
Dolphin does not work with Flash either.
Comment 5 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2013-11-07 08:43:36 PST
We can't dlopen the Flash plugin library on KitKat, and here's the reason:

dlopen failed: cannot locate symbol "_ZN7android10VectorImpl19reservedVectorImpl6Ev" referenced by "libflashplayer.so"...

Game over, Flash is dead on KitKat.
Comment 6 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2013-11-07 08:45:49 PST
Uh that symbol is just a stub....so maybe we could fix this....sigh
Comment 7 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2013-11-07 09:18:08 PST
Or we could just load with RTLD_LAZY since it's probably not going to call this?
Comment 8 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2013-11-07 10:53:35 PST
Somehow RTLD_LAZY didn't help. Maybe it's a noop on Android? We fallback from the gecko linker to the Android one for Flash.
Comment 9 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2013-11-07 13:16:51 PST
Hmmm. It looks like we will have some trouble getting a stub in. AFAICT (from here: https://github.com/android/platform_bionic/blob/master/linker/linker.cpp#L488) the linker tries to resolve symbols in the following order:

1) If we don't have DT_SYMBOLIC, look in the main executable. That would be the zygote, I guess, which is not helpful.

2) The local object (so libflashplayer.so in this case)

3) If built with -Bsymbolic, look in the main executable

4) From LD_PRELOADS. This would be great, except I'm pretty sure that list is parsed way before we are up and running.

5) Lastly, from any needed libraries

Unless I'm missing something, the only way to work around this would be to modify the Flash binaries (or recompile from source obviously).
Comment 10 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2013-11-07 13:17:40 PST
Oh, I also confirmed that RTLD_LAZY is in fact noop. I'm going to send flowers to glibc.
Comment 11 User image Jet Villegas (:jet) 2013-11-13 18:00:33 PST
(In reply to Kevin Brosnan [:kbrosnan] from comment #3)
> Jet do you have any Adobe contacts that would know if critical APIs have
> been removed from Android 4.4 that Flash uses?

snorp has already determined that that is the case.
Comment 12 User image Mike Hommey [:glandium] 2013-11-14 17:24:12 PST
Created attachment 832663 [details] [diff] [review]
Hack that should allow flash to load

This should add enough support for text relocations to get going but there are glitches:
- This doesn't handle flushing cpu cache after text relocations, so this is likely to fail on tegra processors. I think most non-tegras should be fine.
- This doesn't change write permissions after processing the relocations, so this is really a bad hack.
- Our linker has the same limitation as bionic's as in it doesn't look elsewhere than in the dependencies. However, we can easily add hooks in http://hg.mozilla.org/mozilla-central/file/7b014f0f3b03/mozglue/linker/CustomElf.cpp#l289 . I do plan to make things nicer in the future, probably by making the linker lookup symbols in mozglue when it looks in libc, but that would require deeper testing, and I think we can leave with a couple more hooks in CustomElf::GetSymbolPtrInDeps in the meantime.
Comment 13 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2013-11-15 07:10:47 PST
I gave this a shot, but it still bails because we have DF_TEXTREL in DT_FLAGS.
Comment 14 User image Mike Hommey [:glandium] 2013-11-15 14:22:48 PST
Created attachment 833169 [details] [diff] [review]
Hack that should allow flash to load

Forgot about DT_TEXTREL
Comment 15 User image Andreas Kloeckner 2013-11-28 10:22:04 PST
Not sure what I did, but "Shockwave Flash 111r115" loads fine for me in Nightly on CyanogenMod the cm-11.0 built a few days ago.
Comment 16 User image Andreas Kloeckner 2013-11-28 10:22:53 PST
Sorry, that should be "Shockwave Flash 11.1 r115"--that's what it says in about:plugins.
Comment 17 User image Eric Appleman 2013-11-28 11:39:59 PST
Flash 11.1.115.81
CM11
Galaxy S3
Firefox stable and Firefox beta

Works For Me
Comment 18 User image Kevin Brosnan [:kbrosnan] 2013-12-02 17:06:56 PST
I would not be surprised to find that Cyanogen or other custom ROMs reverted the change that broke Flash. Please limit discussion of this bug to OEM ROMs.
Comment 19 User image Sajjad Naveed 2013-12-03 03:53:15 PST
Flash seems to work fine with "modified" libflashplayer.so using Dolphin Browser (with JetPack). 

And although it does appear as a plugin in about:plugins in Firefox. It still doesn't render the flash content.

Here's the post on XDA:

http://forum.xda-developers.com/showthread.php?t=2548001

Direct link to modified flash apk:

https://docs.google.com/file/d/0B1qjrD8ZER9ITmlVNW1EVWM5YlE/edit?pli=1
Comment 20 User image Sajjad Naveed 2013-12-03 03:56:02 PST
Also, it gets to CreatePlugin method in nsNPAPIPlugin but returns here:

if (!pluginLib->HasRequiredFunctions()) {
    NS_WARNING("Not all necessary functions exposed by plugin, it will not load.");
    return NS_ERROR_FAILURE;
  }
Comment 21 User image Sajjad Naveed 2013-12-03 04:09:05 PST
And one more thing in modified libflashplayer.so:

_ZN7android10VectorImpl19reservedVectorImpl6Ev (is replaced with) _ZNK7android10VectorImpl8capacityEv

I think we should be able to get it working on Firefox now as it seems to work fine on "WebKit" (JetPack).
Comment 22 User image Lukas Blakk [:lsblakk] use ?needinfo 2013-12-05 14:44:09 PST
Putting this in the Known Issues for FF26.0
Comment 23 User image ChaChon 2013-12-30 16:14:48 PST
Flash works on firefox for android. but I also reversing flash player apk.

then flash works on firefox and also dolphin jetpack.

here's my modified flash apk link.

https://drive.google.com/file/d/0B2bTyXoO581lYUhQU3pGRmQ4VUE/edit?usp=sharing

Maybe am I late?
Comment 24 User image toquinto253 2013-12-30 16:40:41 PST
Nice works perfectly with the above apk. Appreciate it!
Comment 25 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-01-03 12:48:36 PST
Created attachment 8355639 [details] [diff] [review]
Stub out missing Flash symbols on Android

Mike, I can load Flash on 4.4 now with your hack and this patch.
Comment 26 User image Milan Sreckovic [:milan] 2014-01-03 13:40:06 PST
If there is something that we can do here in the next little while, we should seriously consider it for 27
Comment 27 User image Mike Hommey [:glandium] 2014-01-06 17:36:09 PST
Comment on attachment 8355639 [details] [diff] [review]
Stub out missing Flash symbols on Android

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

::: mozglue/linker/CustomElf.cpp
@@ +766,5 @@
>        symptr = GetSymbolPtrInDeps(strtab.GetStringAt(sym.st_name));
>  
>      if (symptr == nullptr) {
> +      const char *missing_name = strtab.GetStringAt(sym.st_name);
> +      if (ShouldStubMissing(missing_name)) {

This is the wrong place to do this. Just do it in GetSymbolPtrInDeps, although i'm kind of reluctant to add such specific hacks. I'd rather add the symbols as stubs into libmozglue, and have the linker pick them from there, but that's more involved so let's say i'm okay with the hardcoded string checks for now.
Comment 28 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-01-07 07:33:24 PST
Created attachment 8356588 [details] [diff] [review]
Stub out missing Flash symbols
Comment 29 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-01-07 07:35:28 PST
Created attachment 8356589 [details] [diff] [review]
Stub out missing Flash symbols
Comment 30 User image Mike Hommey [:glandium] 2014-01-07 15:31:58 PST
Comment on attachment 8356589 [details] [diff] [review]
Stub out missing Flash symbols

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

::: dom/plugins/base/nsPluginHost.cpp
@@ +2633,5 @@
>      mCachedPlugins = tag;
>    }
>  
> +// On Android we always want to try to load a plugin again (Flash). Bug 935676.
> +#ifndef MOZ_WIDGET_ANDROID

Actually, why is that needed?

(Note for when landing: please land the mozglue/linker part separately, it makes cherry-picking easier to keep the github project up-to-date)

::: mozglue/linker/CustomElf.cpp
@@ +365,5 @@
> +  if (strncmp(symbol,
> +              MISSING_FLASH_SYMNAME_START,
> +              sizeof(MISSING_FLASH_SYMNAME_START) - 1) == 0) {
> +    return FunctionPtr(__flash_stub);
> +  }

I'd rather see this done before symbol resolution in the dependent libraries, like the other hooks, because whatever long term solution will be in place, that's likely to be what it will end up doing. Hopefully, this wouldn't break flash on < 4.4.

Also, the stub is pretty generic, it doesn't need to have flash in its name.
Comment 31 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-01-08 07:30:10 PST
(In reply to Mike Hommey [:glandium] from comment #30)
> Comment on attachment 8356589 [details] [diff] [review]
> Stub out missing Flash symbols
> 
> Review of attachment 8356589 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/plugins/base/nsPluginHost.cpp
> @@ +2633,5 @@
> >      mCachedPlugins = tag;
> >    }
> >  
> > +// On Android we always want to try to load a plugin again (Flash). Bug 935676.
> > +#ifndef MOZ_WIDGET_ANDROID
> 
> Actually, why is that needed?

Because if a prior version of Firefox tried and failed to load the Flash plugin, it would go into the invalid list and never be tried again (unless it was updated or something).

> 
> (Note for when landing: please land the mozglue/linker part separately, it
> makes cherry-picking easier to keep the github project up-to-date)

Yeah, I'll split that part off and get separate review.

> 
> ::: mozglue/linker/CustomElf.cpp
> @@ +365,5 @@
> > +  if (strncmp(symbol,
> > +              MISSING_FLASH_SYMNAME_START,
> > +              sizeof(MISSING_FLASH_SYMNAME_START) - 1) == 0) {
> > +    return FunctionPtr(__flash_stub);
> > +  }
> 
> I'd rather see this done before symbol resolution in the dependent
> libraries, like the other hooks, because whatever long term solution will be
> in place, that's likely to be what it will end up doing. Hopefully, this
> wouldn't break flash on < 4.4.
> 
> Also, the stub is pretty generic, it doesn't need to have flash in its name.

Fair enough, fixed.
Comment 32 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-01-08 07:32:05 PST
Created attachment 8357169 [details] [diff] [review]
Stub out missing Flash symbols
Comment 33 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-01-08 07:39:43 PST
Created attachment 8357175 [details] [diff] [review]
Always try to reload invlalid plugins on Android
Comment 34 User image Mike Hommey [:glandium] 2014-01-10 01:28:08 PST
Comment on attachment 8357169 [details] [diff] [review]
Stub out missing Flash symbols

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

Last nits

::: mozglue/linker/CustomElf.cpp
@@ +282,5 @@
>              reinterpret_cast<const void *>(this), GetPath(), symbol, ptr);
>    return ptr;
>  }
>  
> +#define MISSING_FLASH_SYMNAME_START "_ZN7android10VectorImpl19reservedVectorImpl"

Place that near the strncmp.

@@ +287,5 @@
> +
> +static void
> +__void_stub(void)
> +{
> +}

Enclose in namespace {} /* anonymous namespace */ and remove static (you may just want to put in one of the existing ones).
Comment 35 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-01-10 07:10:11 PST
Created attachment 8358404 [details] [diff] [review]
Stub out missing Flash symbols
Comment 38 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-01-17 11:12:30 PST
Comment on attachment 8358404 [details] [diff] [review]
Stub out missing Flash symbols

[Approval Request Comment]
Bug caused by (feature/regressing bug #): KitKat release
User impact if declined: Flash doesn't work on KitKat
Testing completed (on m-c, etc.): m-c for a few days
Risk to taking this patch (and alternatives if risky): Low
String or IDL/UUID changes made by this patch: None
Comment 40 User image Flaviu Cos 2014-01-21 08:38:31 PST
I tested using Google Nexus 7 (Android 4.4.2).
The flash content stats to play but after a few seconds firefox stops responding (check the screen capture). 
Tested on desktop version of youtube and http://people.mozilla.org/~mwargers/tests/flash/
Comment 41 User image Flaviu Cos 2014-01-21 08:38:56 PST
Created attachment 8363024 [details]
The file  is a screen capture
Comment 42 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-01-21 10:50:49 PST
(In reply to flaviu.cos from comment #40)
> I tested using Google Nexus 7 (Android 4.4.2).
> The flash content stats to play but after a few seconds firefox stops
> responding (check the screen capture). 
> Tested on desktop version of youtube and
> http://people.mozilla.org/~mwargers/tests/flash/

We're tracking the hang in bug 957694. That said, I can definitely reproduce with your test.
Comment 43 User image Pykler 2014-01-26 00:08:51 PST
How do I install firefox with this fix included? Is it in the nightly build yet?
Comment 44 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-01-26 09:46:54 PST
(In reply to Pykler from comment #43)
> How do I install firefox with this fix included? Is it in the nightly build
> yet?

Yes, it's in Nightly, and I think Aurora. It will be in the next beta and also Firefox 27.
Comment 45 User image Pykler 2014-01-26 12:35:52 PST
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) from comment #44)
> (In reply to Pykler from comment #43)
> > How do I install firefox with this fix included? Is it in the nightly build
> > yet?
> 
> Yes, it's in Nightly, and I think Aurora. It will be in the next beta and
> also Firefox 27.

Does not seem to be working for me on Nexus 5 ... do I need to install the "hacked flash" or the standard flash player from adobe?
Comment 46 User image toquinto253 2014-01-26 12:59:52 PST
Yeah also not working on Nexus 7 2013 Rooted running Kitkat 4.4. Tried Nightly and Aurora.
Comment 47 User image ferromvel 2014-01-26 13:14:47 PST
Nexus 7 2013 4.4 with Firefox Beta and standard flash is working for me.
Comment 48 User image toquinto253 2014-01-26 16:40:04 PST
Hmm not sure what is causing the problem then. Flash works in dolphin and boat browser for me just not firefox currently.
Comment 49 User image Aaron Train [:aaronmt] 2014-01-26 16:59:22 PST
(In reply to toquinto253 from comment #46)
> Yeah also not working on Nexus 7 2013 Rooted running Kitkat 4.4. Tried
> Nightly and Aurora.

Are you running a factory image of Android 4.4 or Cyanogenmod or another ROM?
Comment 50 User image Aaron Train [:aaronmt] 2014-01-26 17:00:12 PST
(In reply to Aaron Train [:aaronmt] from comment #49)
> (In reply to toquinto253 from comment #46)
> > Yeah also not working on Nexus 7 2013 Rooted running Kitkat 4.4. Tried
> > Nightly and Aurora.
> 
> Are you running a factory image of Android 4.4 or Cyanogenmod or another ROM?

Also, double check that you're running Flash Player 11.1 for Android 4.0 (11.1.115.81) from http://helpx.adobe.com/flash-player/kb/archived-flash-player-versions.html#main_Archived_versions
Comment 51 User image Pykler 2014-01-26 17:16:12 PST
Works for me on aurora, with the stock flash (not the hacked version used by dolphin). Wondering how do I check if the change went to beta, and then to mainline? I could not find a decent changelog that had this change listed.
Comment 52 User image toquinto253 2014-01-26 18:55:41 PST
Running latest Clean Rom which isn't the problem because i can play flash in Boat browser and dolphin no problem and yeah I'm running the latest stock flash player.
Comment 53 User image Aaron Train [:aaronmt] 2014-01-26 20:09:34 PST
(In reply to Pykler from comment #51)
> Works for me on aurora, with the stock flash (not the hacked version used by
> dolphin). Wondering how do I check if the change went to beta, and then to
> mainline? I could not find a decent changelog that had this change listed.

The latest update on Google Play for Firefox Beta contains the set of patches in this bug. Firefox 27 will be released within the week of the 4th of February.
Comment 54 User image Artem Russakovskii 2014-02-04 16:18:34 PST
Any reason why https://www.mozilla.org/en-US/mobile/27.0/releasenotes/ still mentions this as a bug, if it was fixed and made it to the release?
Comment 55 User image Aaron Train [:aaronmt] 2014-02-04 19:34:11 PST
(In reply to Artem Russakovskii from comment #54)
> Any reason why https://www.mozilla.org/en-US/mobile/27.0/releasenotes/ still
> mentions this as a bug, if it was fixed and made it to the release?

Re ^
Comment 56 User image bhavana bajaj [:bajaj] 2014-02-04 19:45:02 PST
(In reply to Aaron Train [:aaronmt] from comment #55)
> (In reply to Artem Russakovskii from comment #54)
> > Any reason why https://www.mozilla.org/en-US/mobile/27.0/releasenotes/ still
> > mentions this as a bug, if it was fixed and made it to the release?
> 
> Re ^

I updated the notes ~15min back, it looks good now.
Comment 57 User image Flaviu Cos 2014-02-11 06:01:14 PST
Flash content is disabled for Tegra devices running KitKat and higher by bug 957694.

The flash content is flickering on Google Nexus 5 (Android 4.4.2).
Tested on desktop version of youtube.

Should I reopen this bug considering that flash does not work properly?
Comment 58 User image James Willcox (:snorp) (jwillcox@mozilla.com) 2014-02-11 06:33:49 PST
(In reply to flaviu.cos from comment #57)
> Flash content is disabled for Tegra devices running KitKat and higher by bug
> 957694.
> 
> The flash content is flickering on Google Nexus 5 (Android 4.4.2).
> Tested on desktop version of youtube.
> 
> Should I reopen this bug considering that flash does not work properly?

No, please open a new bug.
Comment 59 User image Flaviu Cos 2014-02-11 08:17:59 PST
bug 970941 created for the flickering issue.
Comment 60 User image Flaviu Cos 2014-06-04 04:52:03 PDT
Marking the bug as verified considering the following:
- Works as specified most of the devices with Android 4.4 Kitkat;
- Bug 957694 is filled for the Tegra devices running KitKat;
- Bug 970941 is filled for the flickering issue on Nexus 5;

Note You need to log in before you can comment on or make changes to this bug.