Closed Bug 1163664 Opened 5 years ago Closed 4 years ago

Don't check for plugin blocklist state on Android

Categories

(Core :: Plug-ins, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: jchen, Assigned: rbarker)

Details

Attachments

(1 file)

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.
Do you have a stack of this being triggered?

We should still be loading the blocklist to do android addon blocklisting though, right?
Flags: needinfo?(nchen)
(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).
Flags: needinfo?(nchen)
Assignee: nobody → rbarker
Attachment #8613805 - Flags: review?(nchen)
Attachment #8613805 - Flags: review?(joshmoz)
Attachment #8613805 - Flags: review?(nchen) → review+
Attachment #8613805 - Flags: review?(joshmoz) → review?(jmathies)
You've hardcoded disabling the blocklist for plugins on android. Are you sure you want to do this?
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.
Attachment #8613805 - Flags: review?(jmathies) → review+
(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.
Keywords: checkin-needed
(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.
(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.
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.
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) 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.
https://hg.mozilla.org/mozilla-central/rev/82b993933c92
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.