877061 - PluginProvider can't find "Home" plugins on Android

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Richard Newman [:rnewman]]

AddonManager.getAllAddons(...);

=>

WARN addons.manager: Exception calling callback: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/PluginProvider.jsm :: <TOP_LEVEL> :: line 434"  data: no]" {file: "resource://gre/modules/PluginProvider.jsm" line: 434}]


    // Plugins anywhere else in the user's home are in the user scope
    dir = Services.dirsvc.get("Home", Ci.nsIFile);                     <<<<<<<<<<<<<<<<<
    if (path.substring(0, dir.path.length) == dir.path)
      return AddonManager.SCOPE_USER;

Presumably the PluginProvider is overly optimistic in expecting there to be a home directory on Android.

This is blocking Bug 875400, and thus FHR on Android. Can I get review (or even a patch!) for a quick fix here, like an obvious try..catch?

I'd also be happy with a workaround that can safely grab every installed add-on on Android devices.

(Similar to Bug 566787, perhaps?)

Comment 1

[Dave Townsend [:mossop]]

File a patch with the obvious try...catch and I'll r+ it

Comment 2

[Richard Newman [:rnewman]]

Attachment: Proposed patch. v1

Added catch block for the precise error code raised in this case.

I also switched all of the substring== checks for startsWith, because new JS features are awesome.


Incidentally, this was complaining for

  /data/data/com.adobe.flashplayer/lib/libflashplayer.so

Comment 3

[Richard Newman [:rnewman]]

Attachment: Proposed patch. v2

And here's with a qref for the error code safety check I just added. *sigh*

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 755427 [details] [diff] [review]
Proposed patch. v2

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

::: toolkit/mozapps/extensions/PluginProvider.jsm
@@ +426,2 @@
>        return AddonManager.SCOPE_APPLICATION;
> +    }

Nit: Single line blocks don't have bracing in this file

@@ +432,2 @@
>        return AddonManager.SCOPE_PROFILE;
> +    }

Ditto

@@ +437,5 @@
> +    try {
> +      dir = Services.dirsvc.get("Home", Ci.nsIFile);
> +      if (path.startsWith(dir.path)) {
> +        return AddonManager.SCOPE_USER;
> +      }

Ditto

Comment 5

[Richard Newman [:rnewman]]

Related error during shutdown, btw:

05-29 11:30:22.485 E/GeckoConsole(29398): [JavaScript Error: "ERROR addons.manager: Exception calling provider shutdown: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/FileUtils.jsm :: FileUtils_getDir :: line 63"  data: no]" {file: "resource://gre/modules/FileUtils.jsm" line: 63}]

Comment 6

[Richard Newman [:rnewman]]

Comment on attachment 755427 [details] [diff] [review]
Proposed patch. v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Long-standing issue; apparently nobody has ever called AddonManager.getAllAddons() on an Android device.

User impact if declined: 
  Unable to ship FHR on Android.

Testing completed (on m-c, etc.): 
  Just landed on m-c. Tested by hand.

Risk to taking this patch (and alternatives if risky): 
  Very slim: just a catch block.

String or IDL/UUID changes made by this patch:
  None

Comment 7

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3abf58d85bb0

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3abf58d85bb0

Comment 9

[Richard Newman [:rnewman]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/6a355013cc59