920478 - [app manager] if the launch path of an app is missing or invalid, we don't show any warning

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect, P1)

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Print errors when something is wrong with app launch path.

This adds a bunch of tests against the launch path in the validator
and comes with unit tests.

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

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Print errors when something is wrong with app launch path r=paul

Comment 3

[Alexandre Poirot [:ochameau]]

Comment on attachment 816491 [details] [diff] [review]
Print errors when something is wrong with app launch path r=paul

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

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

::: browser/devtools/app-manager/app-validator.js
@@ +54,5 @@
>      this.error(strings.formatStringFromName("validator.invalidManifestURL", [manifestURL], 1));
>      deferred.resolve(null);
>      return deferred.promise;
>    }
> +  req.channel.loadFlags |= Ci.nsIRequest.LOAD_BYPASS_CACHE | Ci.nsIRequest.INHIBIT_CACHING;

This additional flag was required for tests to behave sanely. So it may fix bug 925219.

Comment 4

[Paul Rouget [:paul]]

Comment on attachment 816491 [details] [diff] [review]
Print errors when something is wrong with app launch path r=paul

> +  req.channel.loadFlags |= Ci.nsIRequest.LOAD_BYPASS_CACHE | Ci.nsIRequest.INHIBIT_CACHING;


This is important for bug 925219 and we need it in aurora.
But we can't land the whole patch in Aurora (strings).

Can you fix that in bug 925219, or have 2 different patches in this bug?

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Factored out the cache fix

Comment 6

[Alexandre Poirot [:ochameau]]

Attachment: Rebased due to bug 926332 updates

I had to modify bug 926332 per review comments.
The getTestFile method is now getTestFilePath and returns a file path instead of nsIFile.
I also fixed a test failure on windows.

I'm carrying over the review as it is really simple updates, but feel free to take another look at it.

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Rebased to update chrome.ini instead of Makefile.ini

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

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Use valid chrome.ini syntax

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

Comment 9

[Alexandre Poirot [:ochameau]]

Test are green and dependency (bug 926332) is ready to land.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/7063d9f5c1fd

Comment 11

[Matt Brubeck (:mbrubeck)]

Sorry, I had to back out this change because it depends on bug 926332 which was backed out:
https://hg.mozilla.org/integration/fx-team/rev/c31df2ff673f

Comment 12

[Matt Brubeck (:mbrubeck)]

Re-landed:
https://hg.mozilla.org/integration/fx-team/rev/894025edbb7b

Comment 13

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/894025edbb7b