1031874 - Updater fails importing UpdaterHealthProvider.jsm on Android updater-enabled builds

Status: RESOLVED WORKSFORME

(Firefox Health Report Graveyard :: Client: Desktop, defect)

Description

[Richard Newman [:rnewman]]

GeckoConsole( 8981): [JavaScript Error: "NS_ERROR_FILE_NOT_FOUND: Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]" {file: "resource://gre/modules/UpdaterHealthProvider.jsm" line: 13}]

I'm guessing that this is from

toolkit/mozapps/update/nsUpdateService.js
16:Components.utils.import("resource://gre/modules/UpdaterHealthProvider.jsm");


This shouldn't be an unhandled error.

Comment 1

[Richard Newman [:rnewman]]

Greg, could you tackle this?

Comment 2

[Gregory Szorc [:gps]]

So nsUpdateService.js is failing to load because of this? That sounds... very bad.

Comment 3

[Richard Newman [:rnewman]]

(In reply to Gregory Szorc [:gps] from comment #2)
> So nsUpdateService.js is failing to load because of this? That sounds...
> very bad.

I'm not sure if nsUpdateService does anything at all on Android. Release and Beta don't use it, of course, and other builds use UpdateService.java.

The right fix might be to simply omit nsUpdateService from Fennec. But that's pretty broad speculation on my part.

snorp, can you confirm?

Comment 4

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

(In reply to Richard Newman [:rnewman] from comment #3)
> (In reply to Gregory Szorc [:gps] from comment #2)
> > So nsUpdateService.js is failing to load because of this? That sounds...
> > very bad.
> 
> I'm not sure if nsUpdateService does anything at all on Android. Release and
> Beta don't use it, of course, and other builds use UpdateService.java.
> 
> The right fix might be to simply omit nsUpdateService from Fennec. But
> that's pretty broad speculation on my part.
> 
> snorp, can you confirm?

Right, we don't use it for updating at all. If nsUpdateService does things besides updating, though, that could cause problems.

Comment 5

[Robert Strong (they/them - no direct email)]

It used to be used for downloading nightly builds... no idea if it is still used.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

I thought it might be used for handling the "update-timer" component category, but it seems like nsUpdateTimerManager.js is used for that. This old line of code tricked me for a moment:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#75

Comment 7

[Richard Newman [:rnewman]]

*does a test build without the update dir*

Comment 8

[Richard Newman [:rnewman]]

Attachment: Don't ship nsUpdateService on Fennec. v1

Patch for discussion.

Comment 9

[Richard Newman [:rnewman]]

I'm a little confused. Building nsAppRunner fails:


 0:01.87 In file included from /Users/rnewman/moz/hg/fx-team/toolkit/xre/nsAppRunner.cpp:25:0:
 0:01.87 /Users/rnewman/moz/hg/fx-team/toolkit/xre/nsUpdateDriver.h:12:30: fatal error: nsIUpdateService.h: No such file or directory


Which is weird, because nsUpdateDriver.h is guarded so that include should never occur:


#ifdef MOZ_UPDATER
#include "nsIUpdateService.h" 


and toolkit/xre/moz.build doesn't set MOZ_UPDATER for android:


if CONFIG['MOZ_UPDATER'] and CONFIG['MOZ_WIDGET_TOOLKIT'] != 'android':
    DEFINES['MOZ_UPDATER'] = True


Sure enough, there's no MOZ_UPDATER in the build command:


 0:01.89 /Users/rnewman/moz/android/android-ndk-r8e/toolchains/arm-linux-androideabi-4.7/prebuilt/darwin-x86_64/bin/arm-linux-androideabi-g++ -o nsAppRunner.o -c -I../../dist/system_wrappers -include /Users/rnewman/moz/hg/fx-team/config/gcc_hidden.h -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_APP_NAME="fennec" -DMOZ_APP_BASENAME="Fennec" -DMOZ_APP_VERSION="34.0a1" -DOS_TARGET="Android" -DMOZ_WIDGET_TOOLKIT="android" -DANDROID_PACKAGE_NAME="org.mozilla.fennec_aurora" -DTARGET_OS_ABI="Android_arm-eabi-gcc3" -DWRAP_SYSTEM_INCLUDES -DGRE_MILESTONE=34.0a1 -DAPP_VERSION=34.0a1 -DAPP_ID={aa3c5121-dab2-40e2-81ca-7ea25febc110} -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -DNO_NSPR_10_SUPPORT -DTOOLKIT_EM_VERSION="34.0a1" -DGRE_BUILDID=20140804175316 -I/Users/rnewman/moz/hg/fx-team/toolkit/xre -I. -I/Users/rnewman/moz/hg/fx-team/ipc/chromium/src -I/Users/rnewman/moz/hg/fx-team/ipc/glue -I/Users/rnewman/moz/hg/fx-team/toolkit/xre/../profile -I/Users/rnewman/moz/hg/fx-team/config -I/Users/rnewman/moz/hg/fx-team/dom/base -I/Users/rnewman/moz/hg/fx-team/dom/ipc -I/Users/rnewman/moz/hg/fx-team/testing/gtest/mozilla -I/Users/rnewman/moz/hg/fx-team/toolkit/crashreporter -I/Users/rnewman/moz/hg/fx-team/xpcom/build -I/Users/rnewman/moz/hg/fx-team/objdir-droid/ipc/ipdl/_ipdlheaders -I../../dist/include -I/Users/rnewman/moz/hg/fx-team/objdir-droid/dist/include/nspr -I/Users/rnewman/moz/hg/fx-team/objdir-droid/dist/include/nss -fPIC -idirafter /Users/rnewman/moz/android/android-ndk-r8e/platforms/android-9/arch-arm/usr/include -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MP -MF .deps/nsAppRunner.o.pp -idirafter /Users/rnewman/moz/android/android-ndk-r8e/platforms/android-9/arch-arm/usr/include -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Werror=int-to-pointer-cast -Wtype-limits -Wempty-body -Wsign-compare -Wno-invalid-offsetof -mandroid -fno-short-enums -fno-exceptions -Wno-psabi -march=armv7-a -mthumb -mfpu=vfp -mfloat-abi=softfp -mno-unaligned-access -isystem /Users/rnewman/moz/hg/fx-team/build/stlport/stlport -isystem /Users/rnewman/moz/android/android-ndk-r8e/sources/cxx-stl/system/include -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pipe -DNDEBUG -DTRIMMED -g -Os -freorder-blocks -fno-reorder-functions -fomit-frame-pointer -I/Users/rnewman/moz/hg/fx-team/objdir-droid/dist/include/cairo /Users/rnewman/moz/hg/fx-team/toolkit/xre/nsAppRunner.cpp


... so what's going on? Greg?

Comment 10

[Robert Strong (they/them - no direct email)]

Mark, doesn't nightly android still use the app update js component for downloading the update and use its own code for installing the apk?

Comment 11

[Richard Newman [:rnewman]]

The download is performed in Java:

http://hg.mozilla.org/mozilla-central/file/default/mobile/android/base/updater/UpdateService.java#l416

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Richard Newman [:rnewman] from comment #11)
> The download is performed in Java:
> 
> http://hg.mozilla.org/mozilla-central/file/default/mobile/android/base/
> updater/UpdateService.java#l416

That's true, but we still use the MOZ_UPDATER preprocessing directive for a number of things:
http://mxr.mozilla.org/mozilla-central/search?string=MOZ_UPDATER&find=mobile/android

Note that it's enabled in the nightly and aurora configure.sh files, and even used to control the Java updater in UpdateServiceHelper.java

Comment 13

[Robert Strong (they/them - no direct email)]

It used to and still has code specifically for Android after UpdateService.java took over the download task. :(
Mozilla 2.0
http://mxr.mozilla.org/mozilla2.0/source/toolkit/mozapps/update/nsUpdateService.js#99

m-c
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#105

It also appears to reuse the app update config var
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/updater/UpdateServiceHelper.java#113

You can see how the updater binary is not built while still building the app update js component here
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/moz.build

I would prefer it if android didn't reuse MOZ_UPDATER to avoid this type of confusion / headache so it would be a good this for a new one to be added for android. Then android can just check its value here, etc.
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/updater/UpdateServiceHelper.java#113

Android can then remove the define MOZ_UPDATER and the android check here can be removed.
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/moz.build#7

Comment 14

[Richard Newman [:rnewman]]

Attachment: Don't ship nsUpdateService on Fennec. v2

This hack gets things building.

Comment 15

[Richard Newman [:rnewman]]

Comment on attachment 8467483 [details] [diff] [review]
Don't ship nsUpdateService on Fennec. v2

Can we do this to strip out the unneeded code from Fennec, and file a bug to split MOZ_UPDATER into two parts?

Comment 16

[Robert Strong (they/them - no direct email)]

Comment on attachment 8467483 [details] [diff] [review]
Don't ship nsUpdateService on Fennec. v2

Unless I am mistaken this will make it so android doesn't have nsIUpdateTimerManager which is used by much more than app update.

I think all you should need to do is add an exclusion of android as is done here
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/moz.build#7

in this location as well
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/moz.build#33

I'm ok with the changes to nsUpdateDriver.h as long a you've verified all is well with updating on android with those changes.

I'm ok with filing a bug to slit MOZ_UPDATER into two parts.

minusing since I believe that android needs nsIUpdateTimerManager

Comment 17

[Richard Newman [:rnewman]]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #16)

> in this location as well
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/moz.
> build#33

Sure, let's do that. Thanks.

Comment 18

[Richard Newman [:rnewman]]

https://tbpl.mozilla.org/?tree=Try&rev=abe43ab4f915

Comment 19

[Richard Newman [:rnewman]]

Nothing complaining in try. Still need to manually test on a device; troublesome right now, because my updater-including build collides with other Firefoxes on the device. Will wrangle that later in the week.

Comment 20

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #16)

> minusing since I believe that android needs nsIUpdateTimerManager

Indeed. FxAndroid uses it for the Block-list ping and the Add-on updater

Comment 21

[Richard Newman [:rnewman]]

I'm not going to have time to work on this.

Comment 22

[Robert Strong (they/them - no direct email)]

App update no longer uses UpdaterHealthProvider.jsm since bug 1121018 removed it so resolving wfm.

I filed bug 1152634 for removing the use / re-purposing of MOZ_UPDATER which is at least in part the cause of this bug since if it weren't defined the file would not have been included.