This is stage 1 in the plan discussed in bug 715816. The code here will get us updates, automatically updated, but not in a bullet-proof way we need for shipping phones.

This patch works, but needs to be cleaned up, split, and the several bugs it exposed on b2g need to be fixed. We can still land all this code without the dependent bug fixes, because the updater will still apply updates next time gecko is restarted.

Comment on attachment 607501 [details] [diff] [review] patch Review of attachment 607501 [details] [diff] [review]: ----------------------------------------------------------------- Just did a quick drive-by review, but the update part looks mostly ok - just need to use the proper #defines to not break fennec ;) The changes in nsThreadManager/nsAppRunner/nsAppShell are from you debugging shutdwon issues right? ::: toolkit/mozapps/update/nsUpdateService.js @@ -114,5 @@ > > const DIR_UPDATES = "updates"; > const FILE_UPDATE_STATUS = "update.status"; > const FILE_UPDATE_VERSION = "update.version"; > -#ifdef ANDROID I guess the right test is MOZ_WIDGET_ANDROID ::: toolkit/mozapps/update/updater/Makefile.in @@ +100,5 @@ > > +ifeq (gonk,$(MOZ_WIDGET_TOOLKIT)) #{ > +HAVE_PROGRESSUI = 1 > +STL_FLAGS = > +CPPSRCS += progressui_gonk.cpp you did not include this file. (changes are in the progressui_null.cpp)

Don't worry about ifdef'ery, was bulldozing. The new file is an hg cp so bugzilla is probably crapping out.

Note for posterity: even with wifi fix applied, shutting down b2g still takes ~30s. Somethin' misbehavin'. Luckily users won't notice, for now.

Helped me debug some issues. Not critical.

Will investigate landing as much as this as possible initially. Then we need to figure out server side and hook up the update URL.

New options for mozconfig, landing in b2g soon ac_add_options --enable-updater #ac_add_options --enable-update-channel=nightly ac_add_options --enable-update-packaging

Basically all we need is updates and a manifest hosted at app.update.url, and we're good to go.

Comment on attachment 607704 [details] [diff] [review] part 4: Add some more debug logging These changes are fine but since we use the log output in tests I want to verify the tests first. Should be done by tomorrow.

Comment on attachment 607762 [details] [diff] [review] part 5: Enable updater in b2g prefs We no longer use app.update.timer with bug 604804 fixed. Not applicable since you are using http at this stage but there is also bug 544442 that checks the server cert used to serve the xml.

(In reply to Robert Strong [:rstrong] (do not email) from comment #12) > Comment on attachment 607704 [details] [diff] [review] > part 4: Add some more debug logging > > These changes are fine but since we use the log output in tests I want to > verify the tests first. Should be done by tomorrow. OK, thanks for letting me know. Which log output is used? I added a couple more LOG stmts in the first patch too, to the update driver.

(In reply to Robert Strong [:rstrong] (do not email) from comment #13) > Comment on attachment 607762 [details] [diff] [review] > part 5: Enable updater in b2g prefs > > We no longer use app.update.timer with bug 604804 fixed. > > Not applicable since you are using http at this stage but there is also bug > 544442 that checks the server cert used to serve the xml. Removed, thanks. We're only using http for developers, in case you're worried :).

The one from the updater binary... the additional LOG stmts to nsUpdateService.js should be fine. Not worried... just making sure you are aware of them so the update flow for b2g gets all of the security checks that are implemented.

Are the tests you're referring to running on tinderbox? I have a try push going through, will find out from there pretty soon.

Yes and I submitted this change along with another change I needed to put through try

Comment on attachment 607704 [details] [diff] [review] part 4: Add some more debug logging This does fail the tests due to the log file output not being the same as the output expected. http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/test/unit/data/partial_log_failure I'm not sure if there are cases where the tests will hit the different additional logs for individual tests. I wouldn't have a problem with just removing the additional log statements as is done for the platform differences here http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/test/unit/head_update.js.in#1037 >diff --git a/toolkit/mozapps/update/updater/updater.cpp b/toolkit/mozapps/update/updater/updater.cpp >--- a/toolkit/mozapps/update/updater/updater.cpp >+++ b/toolkit/mozapps/update/updater/updater.cpp >@@ -1568,41 +1568,53 @@ UpdateThreadFunc(void *param) > { > // open ZIP archive and process... > int rv; > NS_tchar dataFile[MAXPATHLEN]; > NS_tsnprintf(dataFile, sizeof(dataFile)/sizeof(dataFile[0]), > NS_T("%s/update.mar"), gSourcePath); > > rv = gArchiveReader.Open(dataFile); >+ if (rv != OK) { >+ LOG(("failed to open archive\n")); Please include the failure code. >+ } > > #ifdef MOZ_VERIFY_MAR_SIGNATURE > if (rv == OK) { > rv = gArchiveReader.VerifySignature(); >+ if (rv != OK) { >+ LOG(("failed to verify archive signature\n")); Please include the failure code. >+ } > } > > if (rv == OK) { > NS_tchar updateSettingsPath[MAX_TEXT_LEN]; > NS_tsnprintf(updateSettingsPath, > sizeof(updateSettingsPath) / sizeof(updateSettingsPath[0]), > NS_T("%supdate-settings.ini"), gDestPath); > MARChannelStringTable MARStrings; > if (ReadMARChannelIDs(updateSettingsPath, &MARStrings) != OK) { > // If we can't read from update-settings.ini then we shouldn't impose > // a MAR restriction. Some installations won't even include this file. > MARStrings.MARChannelID[0] = '\0'; > } > > rv = gArchiveReader.VerifyProductInformation(MARStrings.MARChannelID, > MOZ_APP_VERSION); >+ if (rv != OK) { >+ LOG(("failed to verify product information\n")); Please include the failure code. >+ } > } > #endif > > if (rv == OK) { > rv = DoUpdate(); >+ if (rv != OK) { >+ LOG(("failed to DoUpdate()\n")); >+ } Please include the failure code. > gArchiveReader.Close(); > } > > if (rv) { > LOG(("failed: %d\n", rv)); > } > else { > #ifdef XP_MACOSX

Comment on attachment 607762 [details] [diff] [review] part 5: Enable updater in b2g prefs app.update.timer is gone from my local patch. Latest tryserver run looks OK, but needs to go through inbound so need to land asap.

Pushed without logging changes. https://hg.mozilla.org/integration/mozilla-inbound/rev/a0b13802f5d1 https://hg.mozilla.org/integration/mozilla-inbound/rev/5ac6a3d91394 https://hg.mozilla.org/integration/mozilla-inbound/rev/faf8114e0700 https://hg.mozilla.org/integration/mozilla-inbound/rev/41f1b58186b4

https://hg.mozilla.org/mozilla-central/rev/a0b13802f5d1 https://hg.mozilla.org/mozilla-central/rev/5ac6a3d91394 https://hg.mozilla.org/mozilla-central/rev/faf8114e0700 https://hg.mozilla.org/mozilla-central/rev/41f1b58186b4 fwiw, you don't need to set [inbound] in the whiteboard.