935676 - Flash doesn't work on 4.4 KitKat

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Plugins, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

At least on my 2012 Nexus 7 flashed to KitKat, Flash doesn't work and does not appear in about:plugins.

Comment 1

[Aaron Train [:aaronmt]]

Kevin, does this work on 4.4 with the real bits?

Comment 2

[Kevin Brosnan [Ex-Mozilla]]

Confirmed on N5.

Worth a relnote as we are noted for our Flash Player support.

Comment 3

[Kevin Brosnan [Ex-Mozilla]]

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

[Aaron Train [:aaronmt]]

Dolphin does not work with Flash either.

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

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

[James Willcox (:snorp) ([e-mail]) (he/him)]

Uh that symbol is just a stub....so maybe we could fix this....sigh

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Or we could just load with RTLD_LAZY since it's probably not going to call this?

Comment 8

[James Willcox (:snorp) ([e-mail]) (he/him)]

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

[James Willcox (:snorp) ([e-mail]) (he/him)]

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

[James Willcox (:snorp) ([e-mail]) (he/him)]

Oh, I also confirmed that RTLD_LAZY is in fact noop. I'm going to send flowers to glibc.

Comment 11

[Jet Villegas (inactive)]

(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

[Mike Hommey [:glandium]]

Attachment: 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

[James Willcox (:snorp) ([e-mail]) (he/him)]

I gave this a shot, but it still bails because we have DF_TEXTREL in DT_FLAGS.

Comment 14

[Mike Hommey [:glandium]]

Attachment: Hack that should allow flash to load

Forgot about DT_TEXTREL

Comment 15

[Andreas Kloeckner]

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

[Andreas Kloeckner]

Sorry, that should be "Shockwave Flash 11.1 r115"--that's what it says in about:plugins.

Comment 17

[Eric Appleman]

Flash 11.1.115.81
CM11
Galaxy S3
Firefox stable and Firefox beta

Works For Me

Comment 18

[Kevin Brosnan [Ex-Mozilla]]

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

[Sajjad Naveed]

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

[Sajjad Naveed]

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

[Sajjad Naveed]

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

[Lukas Blakk [:lsblakk] use ?needinfo]

Putting this in the Known Issues for FF26.0

Comment 23

[ChaChon]

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

[toquinto253]

Nice works perfectly with the above apk. Appreciate it!

Comment 25

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Stub out missing Flash symbols on Android

Mike, I can load Flash on 4.4 now with your hack and this patch.

Comment 26

[Milan Sreckovic [:milan] (needinfo for best results)]

If there is something that we can do here in the next little while, we should seriously consider it for 27

Comment 27

[Mike Hommey [:glandium]]

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

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Stub out missing Flash symbols

Comment 29

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Stub out missing Flash symbols

Comment 30

[Mike Hommey [:glandium]]

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

[James Willcox (:snorp) ([e-mail]) (he/him)]

(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

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Stub out missing Flash symbols

Comment 33

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Always try to reload invlalid plugins on Android

Comment 34

[Mike Hommey [:glandium]]

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

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Stub out missing Flash symbols

Comment 36

[James Willcox (:snorp) ([e-mail]) (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a79597d4cf6a
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9a54946d793

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a79597d4cf6a
https://hg.mozilla.org/mozilla-central/rev/a9a54946d793

Comment 38

[James Willcox (:snorp) ([e-mail]) (he/him)]

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 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a30a7a20eaf4
https://hg.mozilla.org/releases/mozilla-aurora/rev/448e768c4038

https://hg.mozilla.org/releases/mozilla-beta/rev/88e02ab1cc6c
https://hg.mozilla.org/releases/mozilla-beta/rev/4a8c54e35f0d

Comment 40

[Flaviu Cos]

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

[Flaviu Cos]

Attachment: The file is a screen capture

Comment 42

[James Willcox (:snorp) ([e-mail]) (he/him)]

(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

[Pykler]

How do I install firefox with this fix included? Is it in the nightly build yet?

Comment 44

[James Willcox (:snorp) ([e-mail]) (he/him)]

(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

[Pykler]

(In reply to James Willcox (:snorp) ([e-mail]) 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

[toquinto253]

Yeah also not working on Nexus 7 2013 Rooted running Kitkat 4.4. Tried Nightly and Aurora.

Comment 47

[ferromvel]

Nexus 7 2013 4.4 with Firefox Beta and standard flash is working for me.

Comment 48

[toquinto253]

Hmm not sure what is causing the problem then. Flash works in dolphin and boat browser for me just not firefox currently.

Comment 49

[Aaron Train [:aaronmt]]

(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

[Aaron Train [:aaronmt]]

(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

[Pykler]

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

[toquinto253]

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

[Aaron Train [:aaronmt]]

(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

[Artem Russakovskii]

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

[Aaron Train [:aaronmt]]

(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

[bhavana bajaj [:bajaj]]

(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

[Flaviu Cos]

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

[James Willcox (:snorp) ([e-mail]) (he/him)]

(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

[Flaviu Cos]

bug 970941 created for the flickering issue.

Comment 60

[Flaviu Cos]

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;