869083 - fix OS X apps code to return failure on failure to launch app

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Josh Aas]

Attachment: fix v1.0

I noticed that the success variable for launching an app is being ignored. Upon further inspection it seems like it shouldn't be.

This patch gets rid of the unused variable warning and properly returns success or failure. Updated the JS consumer to handle exceptions properly.

Comment 1

[Josh Aas]

Comment on attachment 745963 [details] [diff] [review]
fix v1.0

Requesting review from Bill Walker, perhaps Bill can find a proper reviewer for the JS code.

Comment 2

[Josh Aas]

Also, this patch switches to using stack-based autorelease pools.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 745963 [details] [diff] [review]
fix v1.0

This looks fine to me, but have you tested what happens when an app fails to launch?

One test would be to comment out the call to -[NSWorkspace launchAppWithBundleIdentifier:...] and always return NS_ERROR_FAILURE from nsMacWebAppUtils::LaunchAppWithIdentifier().  Another would be to comment out the call to mwaUtils.launchAppWithIdentifier(aData.origin) and fake an exception instead.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 745963 [details] [diff] [review]
fix v1.0

Felipe reviewed all the changes to this file that weren't platform-wide refactoring efforts, so he would be a good reviewer for these changes to it.

Comment 6

[Chris Peterson [:cpeterson]]

Comment on attachment 745963 [details] [diff] [review]
fix v1.0

Josh, is this patch ready to land? It was r+'d a month ago. <:)

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 745963 [details] [diff] [review]
fix v1.0

Just set checkin-needed in the future :)

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/8e0fdcc36261

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8e0fdcc36261