From 80b3715d3066844313022f02b6ccf87d57ea04e0 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Wed, 25 Feb 2015 15:14:28 -0600 Subject: [PATCH] Bug 1124326 - Improve packageDir support for Cordova. r=ochameau --- browser/devtools/app-manager/app-validator.js | 27 +++--- .../app-manager/test/test_app_validator.html | 2 +- browser/devtools/webide/content/details.js | 2 + browser/devtools/webide/content/newapp.js | 4 +- browser/devtools/webide/content/webide.js | 12 +-- .../devtools/webide/modules/app-manager.js | 92 +++++++++++-------- browser/devtools/webide/modules/build.js | 52 ++++++----- browser/devtools/webide/test/head.js | 2 + browser/devtools/webide/test/test_build.html | 21 ++--- .../webide/test/test_duplicate_import.html | 5 +- browser/devtools/webide/test/test_import.html | 3 + .../webide/test/test_manifestUpdate.html | 13 +-- .../devtools/webide/test/test_runtime.html | 1 + .../devtools/webide/test/test_telemetry.html | 1 + 14 files changed, 129 insertions(+), 108 deletions(-) diff --git a/browser/devtools/app-manager/app-validator.js b/browser/devtools/app-manager/app-validator.js index f8b05c7464a..0368e9b3d26 100644 --- a/browser/devtools/app-manager/app-validator.js +++ b/browser/devtools/app-manager/app-validator.js @@ -12,8 +12,9 @@ const {Task} = Cu.import("resource://gre/modules/Task.jsm", {}); let XMLHttpRequest = CC("@mozilla.org/xmlextras/xmlhttprequest;1"); let strings = Services.strings.createBundle("chrome://browser/locale/devtools/app-manager.properties"); -function AppValidator(project) { - this.project = project; +function AppValidator({ type, location }) { + this.type = type; + this.location = location; this.errors = []; this.warnings = []; } @@ -27,7 +28,7 @@ AppValidator.prototype.warning = function (message) { }; AppValidator.prototype._getPackagedManifestFile = function () { - let manifestFile = FileUtils.File(this.project.location); + let manifestFile = FileUtils.File(this.location); if (!manifestFile.exists()) { this.error(strings.GetStringFromName("validator.nonExistingFolder")); return null; @@ -149,12 +150,12 @@ AppValidator.prototype._fetchManifest = function (manifestURL) { AppValidator.prototype._getManifest = function () { let manifestURL; - if (this.project.type == "packaged") { + if (this.type == "packaged") { manifestURL = this._getPackagedManifestURL(); if (!manifestURL) return promise.resolve(null); - } else if (this.project.type == "hosted") { - manifestURL = this.project.location; + } else if (this.type == "hosted") { + manifestURL = this.location; try { Services.io.newURI(manifestURL, null, null); } catch(e) { @@ -162,7 +163,7 @@ AppValidator.prototype._getManifest = function () { return promise.resolve(null); } } else { - this.error(strings.formatStringFromName("validator.invalidProjectType", [this.project.type], 1)); + this.error(strings.formatStringFromName("validator.invalidProjectType", [this.type], 1)); return promise.resolve(null); } return this._fetchManifest(manifestURL); @@ -181,11 +182,11 @@ AppValidator.prototype.validateManifest = function (manifest) { }; AppValidator.prototype._getOriginURL = function () { - if (this.project.type == "packaged") { + if (this.type == "packaged") { let manifestURL = Services.io.newURI(this.manifestURL, null, null); return Services.io.newURI(".", null, manifestURL).spec; - } else if (this.project.type == "hosted") { - return Services.io.newURI(this.project.location, null, null).prePath; + } else if (this.type == "hosted") { + return Services.io.newURI(this.location, null, null).prePath; } }; @@ -203,9 +204,9 @@ AppValidator.prototype.validateLaunchPath = function (manifest) { } let origin = this._getOriginURL(); let path; - if (this.project.type == "packaged") { + if (this.type == "packaged") { path = "." + ( manifest.launch_path || "/index.html" ); - } else if (this.project.type == "hosted") { + } else if (this.type == "hosted") { path = manifest.launch_path || "/"; } let indexURL; @@ -251,7 +252,7 @@ AppValidator.prototype.validateType = function (manifest) { let appType = manifest.type || "web"; if (["web", "trusted", "privileged", "certified"].indexOf(appType) === -1) { this.error(strings.formatStringFromName("validator.invalidAppType", [appType], 1)); - } else if (this.project.type == "hosted" && + } else if (this.type == "hosted" && ["certified", "privileged"].indexOf(appType) !== -1) { this.error(strings.formatStringFromName("validator.invalidHostedPriviledges", [appType], 1)); } diff --git a/browser/devtools/app-manager/test/test_app_validator.html b/browser/devtools/app-manager/test/test_app_validator.html index e9376d644a8..9e68abc747d 100644 --- a/browser/devtools/app-manager/test/test_app_validator.html +++ b/browser/devtools/app-manager/test/test_app_validator.html @@ -109,7 +109,7 @@ let validator = createPackaged("wrong-launch-path"); validator.validate().then(() => { is(validator.errors.length, 1, "app with wrong path got an error"); - let file = nsFile(validator.project.location); + let file = nsFile(validator.location); file.append("wrong-path.html"); let url = Services.io.newFileURI(file); is(validator.errors[0], strings.formatStringFromName("validator.accessFailedLaunchPath", [url.spec], 1), diff --git a/browser/devtools/webide/content/details.js b/browser/devtools/webide/content/details.js index 79739b67724..b3215e1d2da 100644 --- a/browser/devtools/webide/content/details.js +++ b/browser/devtools/webide/content/details.js @@ -129,6 +129,8 @@ function updateUI() { warningsNode.appendChild(li); } } + + AppManager.update("details"); } function showPrepackageLog() { diff --git a/browser/devtools/webide/content/newapp.js b/browser/devtools/webide/content/newapp.js index 60151c0e891..31ab8802eef 100644 --- a/browser/devtools/webide/content/newapp.js +++ b/browser/devtools/webide/content/newapp.js @@ -153,11 +153,11 @@ function doOK() { target.remove(false); AppProjects.addPackaged(folder).then((project) => { window.arguments[0].location = project.location; - AppManager.validateProject(project).then(() => { + AppManager.validateAndUpdateProject(project).then(() => { if (project.manifest) { project.manifest.name = projectName; AppManager.writeManifest(project).then(() => { - AppManager.validateProject(project).then( + AppManager.validateAndUpdateProject(project).then( () => {window.close()}, bail) }, bail) } else { diff --git a/browser/devtools/webide/content/webide.js b/browser/devtools/webide/content/webide.js index 2d4660e7c3c..c97156d35b8 100644 --- a/browser/devtools/webide/content/webide.js +++ b/browser/devtools/webide/content/webide.js @@ -134,7 +134,7 @@ let UI = { AppManager.selectedProject.type != "mainProcess" && AppManager.selectedProject.type != "runtimeApp" && AppManager.selectedProject.type != "tab") { - AppManager.validateProject(AppManager.selectedProject); + AppManager.validateAndUpdateProject(AppManager.selectedProject); } // Hook to display promotional Developer Edition doorhanger. Only displayed once. @@ -567,7 +567,7 @@ let UI = { menuindex: 1 }); this.projecteditor.on("onEditorSave", (editor, resource) => { - AppManager.validateProject(AppManager.selectedProject); + AppManager.validateAndUpdateProject(AppManager.selectedProject); }); return this.projecteditor.loaded; }, @@ -667,9 +667,6 @@ let UI = { } } - // Validate project - yield AppManager.validateProject(project); - // Select project AppManager.selectedProject = project; }), @@ -1045,9 +1042,6 @@ let Cmds = { // Retrieve added project let project = AppProjects.get(ret.location); - // Validate project - yield AppManager.validateProject(project); - // Select project AppManager.selectedProject = project; @@ -1106,7 +1100,7 @@ let Cmds = { // The result of the validation process (storing names, icons, …) is not stored in // the IndexedDB database when App Manager v1 is used. // We need to run the validation again and update the name and icon of the app. - AppManager.validateProject(project).then(() => { + AppManager.validateAndUpdateProject(project).then(() => { panelItemNode.setAttribute("label", project.name); panelItemNode.setAttribute("image", project.icon); }); diff --git a/browser/devtools/webide/modules/app-manager.js b/browser/devtools/webide/modules/app-manager.js index 9c78c6973ae..42233a2d50f 100644 --- a/browser/devtools/webide/modules/app-manager.js +++ b/browser/devtools/webide/modules/app-manager.js @@ -278,36 +278,36 @@ let AppManager = exports.AppManager = { }, _selectedProject: null, - set selectedProject(value) { + set selectedProject(project) { // A regular comparison still sees a difference when equal in some cases - if (JSON.stringify(this._selectedProject) !== - JSON.stringify(value)) { - - let cancelled = false; - this.update("before-project", { cancel: () => { cancelled = true; } }); - if (cancelled) { - return; - } - - this._selectedProject = value; - - // Clear out tab store's selected state, if any - this.tabStore.selectedTab = null; - - if (this.selectedProject) { - if (this.selectedProject.type == "packaged" || - this.selectedProject.type == "hosted") { - this.validateProject(this.selectedProject); - } - if (this.selectedProject.type == "tab") { - this.tabStore.selectedTab = this.selectedProject.app; - } - } - - this.update("project"); - - this.checkIfProjectIsRunning(); + if (JSON.stringify(this._selectedProject) === + JSON.stringify(project)) { + return; } + + let cancelled = false; + this.update("before-project", { cancel: () => { cancelled = true; } }); + if (cancelled) { + return; + } + + this._selectedProject = project; + + // Clear out tab store's selected state, if any + this.tabStore.selectedTab = null; + + if (project) { + if (project.type == "packaged" || + project.type == "hosted") { + this.validateAndUpdateProject(project); + } + if (project.type == "tab") { + this.tabStore.selectedTab = project.app; + } + } + + this.update("project"); + this.checkIfProjectIsRunning(); }, get selectedProject() { return this._selectedProject; @@ -323,6 +323,19 @@ let AppManager = exports.AppManager = { return AppProjects.remove(location); }, + packageProject: Task.async(function*(project) { + if (!project) { + return; + } + if (project.type == "packaged" || + project.type == "hosted") { + yield ProjectBuilding.build({ + project: project, + logger: this.update.bind(this, "pre-package") + }); + } + }), + _selectedRuntime: null, set selectedRuntime(value) { this._selectedRuntime = value; @@ -481,12 +494,9 @@ let AppManager = exports.AppManager = { return Task.spawn(function* () { let self = AppManager; - let packageDir = yield ProjectBuilding.build({ - project: project, - logger: self.update.bind(self, "pre-package") - }); - - yield self.validateProject(project); + // Package and validate project + yield self.packageProject(project); + yield self.validateAndUpdateProject(project); if (project.errorsCount > 0) { self.reportError("error_cantInstallValidationErrors"); @@ -501,7 +511,7 @@ let AppManager = exports.AppManager = { let response; if (project.type == "packaged") { - packageDir = packageDir || project.location; + let packageDir = yield ProjectBuilding.getPackageDir(project); console.log("Installing app from " + packageDir); response = yield self._appsFront.installPackaged(packageDir, @@ -557,14 +567,20 @@ let AppManager = exports.AppManager = { /* PROJECT VALIDATION */ - validateProject: function(project) { + validateAndUpdateProject: function(project) { if (!project) { return promise.reject(); } return Task.spawn(function* () { - let validation = new AppValidator(project); + let packageDir = yield ProjectBuilding.getPackageDir(project); + let validation = new AppValidator({ + type: project.type, + // Build process may place the manifest in a non-root directory + location: packageDir + }); + yield validation.validate(); if (validation.manifest) { @@ -584,7 +600,7 @@ let AppManager = exports.AppManager = { let origin = Services.io.newURI(manifestURL.prePath, null, null); project.icon = Services.io.newURI(iconPath, null, origin).spec; } else if (project.type == "packaged") { - let projectFolder = FileUtils.File(project.location); + let projectFolder = FileUtils.File(packageDir); let folderURI = Services.io.newFileURI(projectFolder).spec; project.icon = folderURI + iconPath.replace(/^\/|\\/, ""); } diff --git a/browser/devtools/webide/modules/build.js b/browser/devtools/webide/modules/build.js index 507786c9ea9..83eb30fc1f8 100644 --- a/browser/devtools/webide/modules/build.js +++ b/browser/devtools/webide/modules/build.js @@ -44,15 +44,12 @@ const ProjectBuilding = exports.ProjectBuilding = { let manifest = yield ProjectBuilding.fetchPackageManifest(project); logger("start"); - let packageDir; try { - packageDir = yield this._build(project, manifest, logger); + yield this._build(project, manifest, logger); logger("succeed"); } catch(e) { logger("failed", e); } - - return packageDir; }), _build: Task.async(function* (project, manifest, logger) { @@ -61,6 +58,15 @@ const ProjectBuilding = exports.ProjectBuilding = { let command, cwd, args = [], env = []; + // Copy frequently used env vars + let envService = Cc["@mozilla.org/process/environment;1"].getService(Ci.nsIEnvironment); + ["HOME", "PATH"].forEach(key => { + let value = envService.get(key); + if (value) { + env.push(key + "=" + value); + } + }); + if (typeof(manifest.prepackage) === "string") { command = manifest.prepackage.replace(/%project%/g, project.location); } else if (manifest.prepackage.command) { @@ -69,16 +75,9 @@ const ProjectBuilding = exports.ProjectBuilding = { args = manifest.prepackage.args || []; args = args.map(a => a.replace(/%project%/g, project.location)); - env = manifest.prepackage.env || []; + env = env.concat(manifest.prepackage.env || []); env = env.map(a => a.replace(/%project%/g, project.location)); - // Gaia build system crashes if HOME env variable isn't set... - let envService = Cc["@mozilla.org/process/environment;1"].getService(Ci.nsIEnvironment); - let home = envService.get("HOME"); - if (home) { - env.push("HOME=" + home); - } - if (manifest.prepackage.cwd) { // Normalize path for Windows support (converts / to \) let path = OS.Path.normalize(manifest.prepackage.cwd); @@ -110,7 +109,6 @@ const ProjectBuilding = exports.ProjectBuilding = { // Otherwise, on Linux and Mac, SHELL env variable should refer to // the user chosen shell program. // (We do not check for OS, as on windows, with cygwin, ComSpec isn't set) - let envService = Cc["@mozilla.org/process/environment;1"].getService(Ci.nsIEnvironment); let shell = envService.get("ComSpec") || envService.get("SHELL"); args.unshift(command); @@ -155,16 +153,22 @@ const ProjectBuilding = exports.ProjectBuilding = { throw new Error("Unable to run pre-package command '" + command + "' " + args.join(" ") + ":\n" + (e.message || e)); } - - if (manifest.packageDir) { - let packageDir = OS.Path.join(project.location, manifest.packageDir); - // On Windows, replace / by \\ - packageDir = OS.Path.normalize(packageDir); - let exists = yield OS.File.exists(packageDir); - if (exists) { - return packageDir; - } - throw new Error("Unable to resolve application package directory: '" + manifest.packageDir + "'"); - } }), + + getPackageDir: Task.async(function*(project) { + let manifest = yield ProjectBuilding.fetchPackageManifest(project); + if (!manifest || !manifest.webide || !manifest.webide.packageDir) { + return project.location; + } + manifest = manifest.webide; + + let packageDir = OS.Path.join(project.location, manifest.packageDir); + // On Windows, replace / by \\ + packageDir = OS.Path.normalize(packageDir); + let exists = yield OS.File.exists(packageDir); + if (exists) { + return packageDir; + } + throw new Error("Unable to resolve application package directory: '" + manifest.packageDir + "'"); + }) }; diff --git a/browser/devtools/webide/test/head.js b/browser/devtools/webide/test/head.js index 7953a766738..a69c2ef855c 100644 --- a/browser/devtools/webide/test/head.js +++ b/browser/devtools/webide/test/head.js @@ -103,8 +103,10 @@ function nextTick() { } function waitForUpdate(win, update) { + info("Wait: " + update); let deferred = promise.defer(); win.AppManager.on("app-manager-update", function onUpdate(e, what) { + info("Got: " + what); if (what !== update) { return; } diff --git a/browser/devtools/webide/test/test_build.html b/browser/devtools/webide/test/test_build.html index a43a2c3f3fa..3892b9037dc 100644 --- a/browser/devtools/webide/test/test_build.html +++ b/browser/devtools/webide/test/test_build.html @@ -26,7 +26,7 @@ let AppManager = win.AppManager; function isProjectMarkedAsValid() { - let details = win.UI.projecteditor.window.frames[0]; + let details = win.frames[0]; return !details.document.body.classList.contains("error"); } @@ -40,13 +40,10 @@ let packagedAppLocation = getTestFilePath("build_app" + testSuffix + "1"); yield win.Cmds.importPackagedApp(packagedAppLocation); + yield waitForUpdate(win, "details"); let project = win.AppManager.selectedProject; - let deferred = promise.defer(); - win.UI.projecteditor.once("onEditorCreated", deferred.resolve); - yield deferred.promise; - ok(!project.manifest, "manifest includes name"); is(project.name, "--", "Display name uses manifest name"); @@ -55,18 +52,19 @@ loggedMessages.push(msg); } - let packageDir = yield ProjectBuilding.build({ + yield ProjectBuilding.build({ project, logger }); - ok(!packageDir, "no custom packagedir"); + let packageDir = yield ProjectBuilding.getPackageDir(project); + is(packageDir, packagedAppLocation, "no custom packagedir"); is(loggedMessages[0], "start", "log messages are correct"); ok(loggedMessages[1].indexOf("Running pre-package hook") != -1, "log messages are correct"); is(loggedMessages[2], "Terminated with error code: 0", "log messages are correct"); is(loggedMessages[3], "succeed", "log messages are correct"); // Trigger validation - yield AppManager.validateProject(AppManager.selectedProject); + yield AppManager.validateAndUpdateProject(AppManager.selectedProject); yield nextTick(); ok("name" in project.manifest, "manifest includes name"); @@ -79,14 +77,16 @@ packagedAppLocation = getTestFilePath("build_app" + testSuffix + "2"); yield win.Cmds.importPackagedApp(packagedAppLocation); + yield waitForUpdate(win, "project-validated"); project = win.AppManager.selectedProject; loggedMessages = []; - packageDir = yield ProjectBuilding.build({ + yield ProjectBuilding.build({ project, logger }); + packageDir = yield ProjectBuilding.getPackageDir(project); is(OS.Path.normalize(packageDir), OS.Path.join(packagedAppLocation, "stage"), "custom packagedir"); is(loggedMessages[0], "start", "log messages are correct"); @@ -96,6 +96,7 @@ // Switch to the package dir in order to verify the generated webapp.manifest yield win.Cmds.importPackagedApp(packageDir); + yield waitForUpdate(win, "project-validated"); project = win.AppManager.selectedProject; @@ -115,5 +116,3 @@ - - diff --git a/browser/devtools/webide/test/test_duplicate_import.html b/browser/devtools/webide/test/test_duplicate_import.html index df8166f2230..1efcbabe7dd 100644 --- a/browser/devtools/webide/test/test_duplicate_import.html +++ b/browser/devtools/webide/test/test_duplicate_import.html @@ -28,14 +28,17 @@ info("to call importPackagedApp(" + packagedAppLocation + ")"); yield win.Cmds.importPackagedApp(packagedAppLocation); + yield waitForUpdate(win, "project-validated"); yield nextTick(); info("to call importHostedApp(" + hostedAppManifest + ")"); yield win.Cmds.importHostedApp(hostedAppManifest); + yield waitForUpdate(win, "project-validated"); yield nextTick(); info("to call importPackagedApp(" + packagedAppLocation + ") again"); yield win.Cmds.importPackagedApp(packagedAppLocation); + yield waitForUpdate(win, "project-validated"); let project = win.AppManager.selectedProject; is(project.location, packagedAppLocation, "Correctly reselected existing packaged app."); @@ -43,6 +46,7 @@ info("to call importHostedApp(" + hostedAppManifest + ") again"); yield win.Cmds.importHostedApp(hostedAppManifest); + yield waitForUpdate(win, "project-validated"); project = win.AppManager.selectedProject; is(project.location, hostedAppManifest, "Correctly reselected existing hosted app."); yield nextTick(); @@ -71,4 +75,3 @@ - diff --git a/browser/devtools/webide/test/test_import.html b/browser/devtools/webide/test/test_import.html index 6dc5fcddbdb..01b4bf8c78c 100644 --- a/browser/devtools/webide/test/test_import.html +++ b/browser/devtools/webide/test/test_import.html @@ -29,6 +29,7 @@ ok(!win.UI._busyPromise, "UI is not busy"); yield win.Cmds.importPackagedApp(packagedAppLocation); + yield waitForUpdate(win, "project-validated"); let project = win.AppManager.selectedProject; is(project.location, packagedAppLocation, "Location is valid"); @@ -40,6 +41,7 @@ let hostedAppManifest = TEST_BASE + "hosted_app.manifest"; yield win.Cmds.importHostedApp(hostedAppManifest); + yield waitForUpdate(win, "project-validated"); project = win.AppManager.selectedProject; is(project.location, hostedAppManifest, "Location is valid"); @@ -49,6 +51,7 @@ hostedAppManifest = TEST_BASE + "/app"; yield win.Cmds.importHostedApp(hostedAppManifest); + yield waitForUpdate(win, "project-validated"); project = win.AppManager.selectedProject; ok(project.location.endsWith('manifest.webapp'), "The manifest was found and the project was updated"); diff --git a/browser/devtools/webide/test/test_manifestUpdate.html b/browser/devtools/webide/test/test_manifestUpdate.html index 23375e3f8b5..ba74e04d5c3 100644 --- a/browser/devtools/webide/test/test_manifestUpdate.html +++ b/browser/devtools/webide/test/test_manifestUpdate.html @@ -25,20 +25,17 @@ let AppManager = win.AppManager; function isProjectMarkedAsValid() { - let details = win.UI.projecteditor.window.frames[0]; + let details = win.frames[0]; return !details.document.body.classList.contains("error"); } let packagedAppLocation = getTestFilePath("app"); yield win.Cmds.importPackagedApp(packagedAppLocation); + yield waitForUpdate(win, "details"); let project = win.AppManager.selectedProject; - let deferred = promise.defer(); - win.UI.projecteditor.once("onEditorCreated", deferred.resolve); - yield deferred.promise; - ok("name" in project.manifest, "manifest includes name"); is(project.name, project.manifest.name, "Display name uses manifest name"); ok(isProjectMarkedAsValid(), "project is marked as valid"); @@ -66,7 +63,7 @@ yield OS.File.writeAtomic(manifestPath, data , {tmpPath: manifestPath + ".tmp"}); // Trigger validation - yield AppManager.validateProject(AppManager.selectedProject); + yield AppManager.validateAndUpdateProject(AppManager.selectedProject); yield nextTick(); ok(!("name" in project.manifest), "manifest has been updated"); @@ -78,7 +75,7 @@ yield AppManager.writeManifest(project); // Trigger validation - yield AppManager.validateProject(AppManager.selectedProject); + yield AppManager.validateAndUpdateProject(AppManager.selectedProject); yield nextTick(); ok("name" in project.manifest, "manifest includes name"); @@ -97,5 +94,3 @@ - - diff --git a/browser/devtools/webide/test/test_runtime.html b/browser/devtools/webide/test/test_runtime.html index 895bab4afbf..aefb9043427 100644 --- a/browser/devtools/webide/test/test_runtime.html +++ b/browser/devtools/webide/test/test_runtime.html @@ -67,6 +67,7 @@ let packagedAppLocation = getTestFilePath("app"); yield win.Cmds.importPackagedApp(packagedAppLocation); + yield waitForUpdate(win, "project-validated"); let panelNode = win.document.querySelector("#runtime-panel"); let items = panelNode.querySelectorAll(".runtime-panel-item-usb"); diff --git a/browser/devtools/webide/test/test_telemetry.html b/browser/devtools/webide/test/test_telemetry.html index 163fd67a344..f5b58428e8f 100644 --- a/browser/devtools/webide/test/test_telemetry.html +++ b/browser/devtools/webide/test/test_telemetry.html @@ -113,6 +113,7 @@ return Task.spawn(function*() { let packagedAppLocation = getTestFilePath("app"); yield win.Cmds.importPackagedApp(packagedAppLocation); + yield waitForUpdate(win, "project-validated"); }); }