If chrome in desktop mode on Windows 8 defers to Windows 8 metro chrome, then we only want to creat…

If chrome in desktop mode on Windows 8 defers to Windows 8 metro chrome, then we only want to create a new window/incognito
window is one does not exist. To achieve this we look at the startup url being opened and if it is the new tab page, then
we attempt to find an existing browser window and activate it.

For non Windows platforms and Windows desktop mode the behavior is unchanged.

BUG=155134, 155281
R=sky
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=162279

[sky, 2012-10-16 15:46:30]

https://codereview.chromium.org/11137031/diff/3002/chrome/browser/ui/startup/...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://codereview.chromium.org/11137031/diff/3002/chrome/browser/ui/startup/...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:654: if
(PlatformActivateBrowserForProfile(profile_, adjust_urls))
Name this OpenStartupURLsInExistingBrowser and move the implementation to
startup_browser_creator_impl_win.

[ananta, 2012-10-16 18:44:49]

https://codereview.chromium.org/11137031/diff/3002/chrome/browser/ui/startup/...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://codereview.chromium.org/11137031/diff/3002/chrome/browser/ui/startup/...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:654: if
(PlatformActivateBrowserForProfile(profile_, adjust_urls))
On 2012/10/16 15:46:30, sky wrote:
> Name this OpenStartupURLsInExistingBrowser and move the implementation to
> startup_browser_creator_impl_win.

The function does not open the startup urls. It activates the existing browser
instance if the url being opened is the  new tab page. I moved the functionality
to the startup_browser_creator_impl_win.cc file as you suggested. This is
implemented via a static member function
StartupBrowserCreatorImpl::ActivateBrowserForProfile.

[sky, 2012-10-16 21:11:08]

https://codereview.chromium.org/11137031/diff/3002/chrome/browser/ui/startup/...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://codereview.chromium.org/11137031/diff/3002/chrome/browser/ui/startup/...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:654: if
(PlatformActivateBrowserForProfile(profile_, adjust_urls))
On 2012/10/16 18:44:49, ananta wrote:
> On 2012/10/16 15:46:30, sky wrote:
> > Name this OpenStartupURLsInExistingBrowser and move the implementation to
> > startup_browser_creator_impl_win.
> 
> The function does not open the startup urls. It activates the existing browser
> instance if the url being opened is the  new tab page. I moved the
functionality
> to the startup_browser_creator_impl_win.cc file as you suggested. This is
> implemented via a static member function
> StartupBrowserCreatorImpl::ActivateBrowserForProfile.

Logically this method is called during startup to process the startup URLs.
That's why I suggested naming it OpenStartupURLsInExistingBrowser.
PlatformActivateBrowserForProfile is named specified to what we want to do on
windows. You may as well call it ActivateBrowserForProfileOnMetro.
OpenStartupURLsInExistingBrowser is a much more descriptive name.

[ananta, 2012-10-16 21:17:44]

https://codereview.chromium.org/11137031/diff/3002/chrome/browser/ui/startup/...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://codereview.chromium.org/11137031/diff/3002/chrome/browser/ui/startup/...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:654: if
(PlatformActivateBrowserForProfile(profile_, adjust_urls))
On 2012/10/16 21:11:08, sky wrote:
> On 2012/10/16 18:44:49, ananta wrote:
> > On 2012/10/16 15:46:30, sky wrote:
> > > Name this OpenStartupURLsInExistingBrowser and move the implementation to
> > > startup_browser_creator_impl_win.
> > 
> > The function does not open the startup urls. It activates the existing
browser
> > instance if the url being opened is the  new tab page. I moved the
> functionality
> > to the startup_browser_creator_impl_win.cc file as you suggested. This is
> > implemented via a static member function
> > StartupBrowserCreatorImpl::ActivateBrowserForProfile.
> 
> Logically this method is called during startup to process the startup URLs.
> That's why I suggested naming it OpenStartupURLsInExistingBrowser.
> PlatformActivateBrowserForProfile is named specified to what we want to do on
> windows. You may as well call it ActivateBrowserForProfileOnMetro.
> OpenStartupURLsInExistingBrowser is a much more descriptive name.

Fair enough. Done.

[sky, 2012-10-16 22:06:15]

https://codereview.chromium.org/11137031/diff/18003/chrome/browser/ui/startup...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://codereview.chromium.org/11137031/diff/18003/chrome/browser/ui/startup...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:1042: Profile*
profile, const std::vector<GURL>& startup_urls) {
Same thing about wrapping here. Also, why the USE_AURA ifdef? Don't we want this
on windows with aura? I'm assuming at some point we'll have aura working with
metro and then we would want this logic, right?

https://codereview.chromium.org/11137031/diff/18003/chrome/browser/ui/startup...
File chrome/browser/ui/startup/startup_browser_creator_impl.h (right):

https://codereview.chromium.org/11137031/diff/18003/chrome/browser/ui/startup...
chrome/browser/ui/startup/startup_browser_creator_impl.h:144: Profile* profile,
const std::vector<GURL>& startup_urls);
When you wrap wrap each param to a newline.

[ananta, 2012-10-16 22:14:58]

https://codereview.chromium.org/11137031/diff/18003/chrome/browser/ui/startup...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

https://codereview.chromium.org/11137031/diff/18003/chrome/browser/ui/startup...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:1042: Profile*
profile, const std::vector<GURL>& startup_urls) {
On 2012/10/16 22:06:15, sky wrote:
> Same thing about wrapping here. Also, why the USE_AURA ifdef? Don't we want
this
> on windows with aura? I'm assuming at some point we'll have aura working with
> metro and then we would want this logic, right?

Fixed the wrapping. Regarding AURA this code path is mostly invoked from desktop
chrome which delegates to metro. Under AURA it is very likely that desktop
chrome would put a viewer on similar lines as metro. We can revisit this once we
have details flushed out on that.

https://codereview.chromium.org/11137031/diff/18003/chrome/browser/ui/startup...
File chrome/browser/ui/startup/startup_browser_creator_impl.h (right):

https://codereview.chromium.org/11137031/diff/18003/chrome/browser/ui/startup...
chrome/browser/ui/startup/startup_browser_creator_impl.h:144: Profile* profile,
const std::vector<GURL>& startup_urls);
On 2012/10/16 22:06:15, sky wrote:
> When you wrap wrap each param to a newline.

Done.

[sky, 2012-10-16 23:16:16]

LGTM