1163664 - Don't check for plugin blocklist state on Android

Status: RESOLVED FIXED

(Core :: Plug-ins, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

Fennec doesn't support plugins, but nsIBlocklistService::GetPluginBlocklistState still gets called through analytics scripts, etc. This causes us to load the blocklist, which can take ~1 second on an Android device, affecting our pageload performance.

Comment 1

[Benjamin Smedberg]

Do you have a stack of this being triggered?

We should still be loading the blocklist to do android addon blocklisting though, right?

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> Do you have a stack of this being triggered?

Looks like this is only triggered if Flash is installed on the device, which makes more sense and is not as bad as I had thought (even though currently the blocklist is not applicable to Flash on Android). In summary, blocklist loading could slow down pageload by 1s or more, but it only affects those with Flash installed. So this bug is potentially WONTFIX.

The stack is something like,

> navigator.javaEnabled (or navigator.plugins)
> ...
> nsPluginArray::EnsurePlugins
> nsPluginHost::GetPlugins
> nsPluginHost::LoadPlugins
> ...
> nsPluginHost::AddPluginTag
> nsPluginTag::IsActive
> nsPluginTag::IsBlocklisted
> nsPluginTag::GetBlocklistState
> nsBlocklistService.getPluginBlocklistState
> nsBlocklistService._loadBlocklist


> We should still be loading the blocklist to do android addon blocklisting
> though, right?

Right, the blocklist should be loaded when the addons manager needs it (e.g. when installing a new addon).

Comment 3

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1163664-Don-t-check-for-plugin-blocklist-state-on-Android-15060117-6c135d9.patch

Comment 4

[Randall Barker [:rbarker]]

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

Comment 5

[Jim Mathies [:jimm]]

You've hardcoded disabling the blocklist for plugins on android. Are you sure you want to do this?

Comment 6

[Jim Mathies [:jimm]]

Comment on attachment 8613805 [details] [diff] [review]
0001-Bug-1163664-Don-t-check-for-plugin-blocklist-state-on-Android-15060117-6c135d9.patch

patch looks fine.

Comment 7

[Randall Barker [:rbarker]]

(In reply to Jim Mathies [:jimm] from comment #5)
> You've hardcoded disabling the blocklist for plugins on android. Are you
> sure you want to do this?

:snorp said we do since we only support Flash on Android and will never block flash.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/82b993933c92

Comment 9

[Jim Mathies [:jimm]]

(In reply to Randall Barker [:rbarker] from comment #7)
> (In reply to Jim Mathies [:jimm] from comment #5)
> > You've hardcoded disabling the blocklist for plugins on android. Are you
> > sure you want to do this?
> 
> :snorp said we do since we only support Flash on Android and will never
> block flash.

Why would we never want to block flash? That's the one plugin we block the most on desktop.

Comment 10

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

(In reply to Jim Mathies [:jimm] from comment #9)
> (In reply to Randall Barker [:rbarker] from comment #7)
> > (In reply to Jim Mathies [:jimm] from comment #5)
> > > You've hardcoded disabling the blocklist for plugins on android. Are you
> > > sure you want to do this?
> > 
> > :snorp said we do since we only support Flash on Android and will never
> > block flash.
> 
> Why would we never want to block flash? That's the one plugin we block the
> most on desktop.

It's unmaintained. If we block it, nobody would ever be able to use it. And that would make those people very angry. We accidentally blocked in the past because 'linux' matches Android as well, and it was not a good thing.

Comment 11

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

I should add, though, that the real reason we want this patch is because loading the stupid blocklist takes OVER ONE SECOND on the first page that queries for plugins.

Comment 12

[Jim Mathies [:jimm]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #11)
> I should add, though, that the real reason we want this patch is because
> loading the stupid blocklist takes OVER ONE SECOND on the first page that
> queries for plugins.

That sounds like a bug worth filing, a 163kb xml file shouldn't take a second to parse.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/82b993933c92