Introduce LaunchParams struct for opening chrome apps.

Introduce LaunchParams struct for opening chrome apps.

This makes it easier to update the parameters used when changing how apps are launched without updating many call sites with unused parameters.

BUG=None
TEST=Covered by automated tests.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=146554

[benwells, 2012-07-09 07:19:34]

I need to add a new parameter to the OpenApplication call, so am making this
change first.

+mihaip for extensions related stuff
+ben as ui owner

[benwells, 2012-07-10 22:29:07]

This time with reviewers added...

I need to add a new parameter to the OpenApplication call, so am making this
change first.

+mihaip for extensions related stuff
+ben as ui owner

[Mihai Parparita -not on Chrome, 2012-07-11 00:38:35]

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/extension...
File chrome/browser/ui/extensions/application_launch.h (right):

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/extension...
chrome/browser/ui/extensions/application_launch.h:57: content::WebContents*
OpenApplicationPanel(
Can these other methods be removed from the header file and into an anonymous
namespace in the .cc file? OpenApplicationPanel is never used outside, and it
seems like the others should end up on the LaunchParams codepath too.

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/extension...
chrome/browser/ui/extensions/application_launch.h:62: // Opens a new application
window for the specified url. If |as_panel| is true,
as_panel seems to no longer exist.

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/extension...
chrome/browser/ui/extensions/application_launch.h:80: //
Browser::OpenApplicationWindow().
OpenApplicationWindow is no longer in Browser.

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/views/ash...
File chrome/browser/ui/views/ash/extension_utils.cc (right):

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/views/ash...
chrome/browser/ui/views/ash/extension_utils.cc:48: params.override_url = url;
Is this actually needed? The only possible value for url is GetFullLaunchURL,
and we already default to that.

[Dmitry Titov, 2012-07-11 01:03:24]

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/extension...
File chrome/browser/ui/extensions/application_launch.h (right):

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/extension...
chrome/browser/ui/extensions/application_launch.h:57: content::WebContents*
OpenApplicationPanel(
On 2012/07/11 00:38:36, Mihai Parparita wrote:
> Can these other methods be removed from the header file and into an anonymous
> namespace in the .cc file? OpenApplicationPanel is never used outside, and it
> seems like the others should end up on the LaunchParams codepath too.


This method is removed in https://chromiumcodereview.appspot.com/10689071/
which I think already landed...

Also, the panel_app_browsertest.cc is removed.

[benwells, 2012-07-12 04:21:02]

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/extension...
File chrome/browser/ui/extensions/application_launch.h (right):

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/extension...
chrome/browser/ui/extensions/application_launch.h:57: content::WebContents*
OpenApplicationPanel(
On 2012/07/11 00:38:36, Mihai Parparita wrote:
> Can these other methods be removed from the header file and into an anonymous
> namespace in the .cc file? OpenApplicationPanel is never used outside, and it
> seems like the others should end up on the LaunchParams codepath too.

Yes indeed, I was thinking the same thing but forgot about it :(

I've moved OpenApplicationTab and OpenApplicationWindow (the patch Dmitry
mentions was reverted, I'm waiting for it to come in before finishing this off.

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/extension...
chrome/browser/ui/extensions/application_launch.h:80: //
Browser::OpenApplicationWindow().
On 2012/07/11 00:38:36, Mihai Parparita wrote:
> OpenApplicationWindow is no longer in Browser.

Done.

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/views/ash...
File chrome/browser/ui/views/ash/extension_utils.cc (right):

http://codereview.chromium.org/10700130/diff/4002/chrome/browser/ui/views/ash...
chrome/browser/ui/views/ash/extension_utils.cc:48: params.override_url = url;
On 2012/07/11 00:38:36, Mihai Parparita wrote:
> Is this actually needed? The only possible value for url is GetFullLaunchURL,
> and we already default to that.

Wow yeah, deleted!

[Mihai Parparita -not on Chrome, 2012-07-12 04:34:20]

LGTM

http://codereview.chromium.org/10700130/diff/39/chrome/browser/ui/extensions/...
File chrome/browser/ui/extensions/application_launch.h (right):

http://codereview.chromium.org/10700130/diff/39/chrome/browser/ui/extensions/...
chrome/browser/ui/extensions/application_launch.h:70: bool update_shortcut);
Unless Code Search is lying, the only non-test callsite to OpenApplicationWindow
is
http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/ui/startup/...,
and that alway passes in true for update_shortcut. Can that parameter be
removed?

[Ben Goodger (Google), 2012-07-12 15:44:24]

lgtm

[commit-bot: I haz the power, 2012-07-13 03:38:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/10700130/7018

[commit-bot: I haz the power, 2012-07-13 03:58:05]

Try job failure for 10700130-7018 (retry) on linux_chromeos for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2012-07-13 04:45:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/10700130/10020

[commit-bot: I haz the power, 2012-07-13 05:12:01]

Try job failure for 10700130-10020 (retry) on linux_chromeos for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2012-07-13 06:49:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/10700130/11004

[commit-bot: I haz the power, 2012-07-13 07:44:31]

Try job failure for 10700130-11004 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-07-13 07:47:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/10700130/11004

[commit-bot: I haz the power, 2012-07-13 08:42:25]

Try job failure for 10700130-11004 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...