Refactor PlatformAppCommandLineLauncher into PlatformAppPathLauncher

Refactor PlatformAppCommandLineLauncher into PlatformAppPathLauncher

(This is an isolated part of change 10834383). This allows platform apps to be launched targeted at specific paths. Previously, this only worked if the path was specified as part of a CommandLine.

BUG=


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=154741

[thorogood, 2012-09-03 07:53:27]

Hey guys, just thought I'd break out a part of my big Chrome OS file picker CL.

[benwells, 2012-09-04 04:41:31]

lgtm except comment nits

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
File chrome/browser/extensions/platform_app_launcher.cc (right):

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
chrome/browser/extensions/platform_app_launcher.cc:73: // Helper method to
launches the platform app |extension| with no data. This
launches => launch

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
chrome/browser/extensions/platform_app_launcher.cc:74: // should be called in
the fallback case for the launchers above.
Above?

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
chrome/browser/extensions/platform_app_launcher.cc:229: // The path to be passed
through to the app. This may be the empty path.
It can't be empty any more.

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
File chrome/browser/extensions/platform_app_launcher.h (right):

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
chrome/browser/extensions/platform_app_launcher.h:30: // Launches the platform
app |extension| at the specific |file_path| given.
at the specific |file_path| given => with the contents of |file_path| available
through the launch data.

[thorogood, 2012-09-04 05:25:50]

Thanks!

On 2012/09/04 04:41:31, benwells wrote:
> lgtm except comment nits
> 
>
https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
> File chrome/browser/extensions/platform_app_launcher.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
> chrome/browser/extensions/platform_app_launcher.cc:73: // Helper method to
> launches the platform app |extension| with no data. This
> launches => launch
> 
>
https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
> chrome/browser/extensions/platform_app_launcher.cc:74: // should be called in
> the fallback case for the launchers above.
> Above?
> 
>
https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
> chrome/browser/extensions/platform_app_launcher.cc:229: // The path to be
passed
> through to the app. This may be the empty path.
> It can't be empty any more.
> 
>
https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
> File chrome/browser/extensions/platform_app_launcher.h (right):
> 
>
https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
> chrome/browser/extensions/platform_app_launcher.h:30: // Launches the platform
> app |extension| at the specific |file_path| given.
> at the specific |file_path| given => with the contents of |file_path|
available
> through the launch data.

[thorogood, 2012-09-04 05:26:00]

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
File chrome/browser/extensions/platform_app_launcher.cc (right):

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
chrome/browser/extensions/platform_app_launcher.cc:73: // Helper method to
launches the platform app |extension| with no data. This
On 2012/09/04 04:41:32, benwells wrote:
> launches => launch

Done.

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
chrome/browser/extensions/platform_app_launcher.cc:74: // should be called in
the fallback case for the launchers above.
On 2012/09/04 04:41:32, benwells wrote:
> Above?

I lazily moved this method to the top of the file without updating the comment.
It's slightly larger now.

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
chrome/browser/extensions/platform_app_launcher.cc:229: // The path to be passed
through to the app. This may be the empty path.
On 2012/09/04 04:41:32, benwells wrote:
> It can't be empty any more.

Done.

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
File chrome/browser/extensions/platform_app_launcher.h (right):

https://chromiumcodereview.appspot.com/10919058/diff/1003/chrome/browser/exte...
chrome/browser/extensions/platform_app_launcher.h:30: // Launches the platform
app |extension| at the specific |file_path| given.
On 2012/09/04 04:41:32, benwells wrote:
> at the specific |file_path| given => with the contents of |file_path|
available
> through the launch data.

Done.

[commit-bot: I haz the power, 2012-09-04 05:51:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thorogood@chromium.org/10919058/3004

[commit-bot: I haz the power, 2012-09-04 05:51:18]

Presubmit check for 10919058-3004 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/extensions

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[koz (OOO until 15th September), 2012-09-04 05:59:51]

lgtm

[commit-bot: I haz the power, 2012-09-04 06:00:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thorogood@chromium.org/10919058/3004

[commit-bot: I haz the power, 2012-09-04 10:07:08]

Change committed as 154741