There are two Marketplace packaged apps in Marketplace: https://marketplace.firefox.com/app/marketplace-package https://marketplace.firefox.com/app/marketplace And both can be installed and used on Android (and presumably FxOS). But when I install and launch them on Mac, they both display a white screen of sorrow. Console output is unilluminative: 06-24 08:56 > /Applications/Marketplace.app/Contents/MacOS/webapprt 2014-06-24 10:59:45.078 webapprt[73373:507] MY WEBAPPRT BUILDID: 20140623030201 2014-06-24 10:59:45.079 webapprt[73373:507] found override firefox binary: org.mozilla.nightly 2014-06-24 10:59:45.081 webapprt[73373:507] USING FIREFOX : /Applications/FirefoxNightly.app 2014-06-24 10:59:45.082 webapprt[73373:507] Looking for firefox ini file here: /Applications/FirefoxNightly.app/Contents/MacOS/application.ini 2014-06-24 10:59:45.082 webapprt[73373:507] FIREFOX WEBAPPRT BUILDID: 20140623030201 2014-06-24 10:59:45.082 webapprt[73373:507] This Application has the newest webrt. Launching! 2014-06-24 10:59:45.083 webapprt[73373:507] Set XUL_APP_FILE to: /Applications/Marketplace.app/Contents/MacOS/webapp.ini 2014-06-24 10:59:45.098 webapprt[73373:507] WebappRT application.ini path: /Applications/FirefoxNightly.app/Contents/MacOS/webapprt/webapprt.ini 2014-06-24 10:59:45.099 webapprt[73373:507] setting app profile: marketplace-48ae30af1a6f84a65338f3e1a91d8a67 !! Creating a dummy channel for {7b914b11-e6f1-a840-ba42-3c525f6859ec} (no appInfo) But, strangely, the app works on a debug build (after I set FirefoxBinary to org.mozilla.nightlydebug in its Info.plist file), although it displays the following error message in the console: JavaScript error: chrome://webapprt/content/webapp.js, line 127: WebappRT.config is undefined And if I then launch the app with my regular nightly build, it continues to work.

I haven't investigated further, but looks like we're trying to get the app info using the ID "{7b914b11-e6f1-a840-ba42-3c525f6859ec}" (or another generated ID), while the app in the registry is stored with the ID "marketplace.firefox.com". It's almost surely due to the ID changing code (probably we're using the old ID while installing the app).

The custom origin was indeed the problem. I'll try to write a test case.

Comment on attachment 8446652 [details] [diff] [review] WIP Review of attachment 8446652 [details] [diff] [review]: ----------------------------------------------------------------- Seems reasonable to me!

Try runs were quite slow today. This is a try run (that was failing on Mac because I forgot to rename a function...): https://tbpl.mozilla.org/?tree=Try&rev=4839f99f9fab This is a Mac-only try run with the patch fixed: https://tbpl.mozilla.org/?tree=Try&rev=d95ee4e2b324 (The Mac run hasn't finished yet, I'll see the results tomorrow)

Now works on Mac too: https://tbpl.mozilla.org/?tree=Try&rev=d93f6e44e254

I forgot to add the files to regenerate the signed app package.

Comment on attachment 8447786 [details] [diff] [review] Patch Review of attachment 8447786 [details] [diff] [review]: ----------------------------------------------------------------- A Security - Mozilla PSM Glue peer will need to review the security/manager/ changes, so requesting review from dkeeler. ::: toolkit/webapps/MacNativeApp.js @@ +33,5 @@ > + * apps. In production code, it's "/Applications". In tests, it's > + * "~/Applications" because on build machines we don't have enough privileges > + * to write to the global "/Applications" directory. > + */ > +NativeApp._rootInstallDir = LOCAL_APP_DIR; This is ok, especially since individual apps never override the value, but I wonder why it's necessary, since the tests could already override the value for all apps simply by modifying NativeApp.prototype._rootInstallDir. ::: toolkit/webapps/tests/head.js @@ +405,5 @@ > + getInterface(Ci.nsIWebNavigation). > + QueryInterface(Ci.nsIDocShell). > + chromeEventHandler.ownerDocument.defaultView. > + PopupNotifications.panel; > + Nit: this line has trailing whitespace. 06-30 09:55 > git apply ~/fix_installation_of_apps_with_custom_origin /Users/myk/fix_installation_of_apps_with_custom_origin:1382: trailing whitespace. warning: 1 line adds whitespace errors. @@ +409,5 @@ > + > + popupPanel.addEventListener("popupshown", function onPopupShown() { > + popupPanel.removeEventListener("popupshown", onPopupShown, false); > + this.childNodes[0].button.doCommand(); > + }, false); See attachment 8445433 [details] [diff] [review] in bug 1000315 for a fix to this implementation of confirmNextInstall! But also see bug 1000315, comment 46 for my question about the fix. ::: toolkit/webapps/tests/test_custom_origin.xul @@ +60,2 @@ > > + TrustedRootCertificate.index = Ci.nsIX509CertDB.AppXPCShellRoot; Nit: elsewhere, you change the value of a property before registering a cleanup function that changes it back, and that seems like a more natural order. @@ +60,5 @@ > > + TrustedRootCertificate.index = Ci.nsIX509CertDB.AppXPCShellRoot; > + > + // Allow signed apps to be installed from the test origin > + let old_signed_app_origins; Nit: I'd still use camelCase names for this variable (and new_signed_app_origins), even though they represent a pref with an underscore_separated_name. @@ +80,5 @@ > } > > + confirmNextInstall(); > + > + let deferredInstall = Promise.defer() Nit: trailing semicolon. @@ +86,5 @@ > + request.onerror = function() { > + deferredInstall.reject(this.error.name); > + }; > + request.onsuccess = deferredInstall.resolve; > + yield deferredInstall.promise; Nice use of a deferred to wrap a callback-based API! @@ +91,5 @@ > + > + let appObject = request.result; > + ok(appObject, "app is non-null"); > + > + let deferredDownload = Promise.defer(); Nit: it might be worth noting here that the download happens automatically, as it was surprising at first to see download handlers being attached without a download being initiated. @@ +96,5 @@ > + appObject.ondownloaderror = function() { > + deferredDownload.reject(this.error.name); > + }; > + appObject.ondownloadapplied = deferredDownload.resolve; > + yield deferredDownload.promise; You could avoid having to come up with unique names for the deferred objects by using a let statement to scope them to the code that uses them: let (deferred = Promise.defer()) { appObject.ondownloaderror = function() { deferred.reject(this.error.name); }; appObject.ondownloadapplied = deferred.resolve; yield deferred.promise; };

Comment on attachment 8447786 [details] [diff] [review] Patch Review of attachment 8447786 [details] [diff] [review]: ----------------------------------------------------------------- The changes to security/manager/... look good to me. ::: security/manager/ssl/tests/unit/test_signed_apps/gentestfiles/create_test_files.sh @@ +169,5 @@ > # to avoid that reverting again > PRIV_UID=`uuidgen` > signApp $DB_PATH ${BASE_PATH}/unsigned_app_origin.zip \ > $TEST_APP_PATH/origin_app_1.zip \ > + $PRIV_UID 1 ${TRUSTED_EE} Oh dear. I guess this was always wrong? @@ +202,2 @@ > echo cp ${TEST_APP_PATH}/* ${TEST_APP_PATH}/../unsigned_app_1.zip ${BASE_PATH}/.. > + echo cp ${TEST_APP_PATH}/* ${BASE_PATH}/${TOOLKIT_WEBAPPS_TEST_LOC} nit: I'm not sure why there are two spaces before ${TEST_APP_PATH}, etc., but only one is necessary @@ +207,5 @@ > done > > cp ${TEST_APP_PATH}/* ${BASE_PATH}/${APPS_TEST_LOC} > cp ${TEST_APP_PATH}/* ${TEST_APP_PATH}/../unsigned_app_1.zip ${BASE_PATH}/.. > +cp ${TEST_APP_PATH}/* ${BASE_PATH}/${TOOLKIT_WEBAPPS_TEST_LOC} nit: only one space is necessary before ${BASE_PATH}

https://tbpl.mozilla.org/?tree=Try&rev=4a1d96139ba0 https://hg.mozilla.org/integration/mozilla-inbound/rev/2ec73f026a68