When user presses Ctrl-N, we should use the current active window's profile to determine on which p…

When user presses Ctrl-N, we should use the current active window's profile to determine on which profile we should open a new window.

BUG=608840
Committed: https://crrev.com/ffeee4dcafd17d3658c3c659cf93aa49dcc90da8
Cr-Commit-Position: refs/heads/master@{#392414}

[xdai1, 2016-05-05 21:09:01]

xdai@chromium.org changed reviewers:
+ oshima@chromium.org

[xdai1, 2016-05-05 21:09:01]

oshima@, could you help review this CL please? Thanks!

[oshima, 2016-05-05 21:38:12]

can you add a test?

[oshima, 2016-05-05 21:38:12]

can you add a test?

[xdai1, 2016-05-06 18:31:24]

Test added. Please take another look, thanks!

[oshima, 2016-05-06 18:44:14]

https://codereview.chromium.org/1954853002/diff/20001/chrome/browser/ui/ash/c...
File chrome/browser/ui/ash/chrome_new_window_delegate.cc (right):

https://codereview.chromium.org/1954853002/diff/20001/chrome/browser/ui/ash/c...
chrome/browser/ui/ash/chrome_new_window_delegate.cc:105: profile =
ProfileManager::GetActiveUserProfile();
optional: conditional operator?

https://codereview.chromium.org/1954853002/diff/20001/chrome/browser/ui/ash/c...
File chrome/browser/ui/ash/chrome_new_window_delegate_browsertest.cc (right):

https://codereview.chromium.org/1954853002/diff/20001/chrome/browser/ui/ash/c...
chrome/browser/ui/ash/chrome_new_window_delegate_browsertest.cc:53:
EXPECT_EQ(chrome::FindBrowserWithWindow(window3)->profile(), profile1);
can you also add the case when the incognito is active?

[xdai1, 2016-05-06 20:25:33]

oshima@, I've addressed the comments. Please take another look, thanks!

https://codereview.chromium.org/1954853002/diff/20001/chrome/browser/ui/ash/c...
File chrome/browser/ui/ash/chrome_new_window_delegate.cc (right):

https://codereview.chromium.org/1954853002/diff/20001/chrome/browser/ui/ash/c...
chrome/browser/ui/ash/chrome_new_window_delegate.cc:105: profile =
ProfileManager::GetActiveUserProfile();
On 2016/05/06 18:44:14, oshima wrote:
> optional: conditional operator? 

Done.

https://codereview.chromium.org/1954853002/diff/20001/chrome/browser/ui/ash/c...
File chrome/browser/ui/ash/chrome_new_window_delegate_browsertest.cc (right):

https://codereview.chromium.org/1954853002/diff/20001/chrome/browser/ui/ash/c...
chrome/browser/ui/ash/chrome_new_window_delegate_browsertest.cc:53:
EXPECT_EQ(chrome::FindBrowserWithWindow(window3)->profile(), profile1);
On 2016/05/06 18:44:14, oshima wrote:
> can you also add the case when the incognito is active?

Done.

[oshima, 2016-05-06 20:53:36]

lgtm thanks!

[xdai1, 2016-05-07 04:53:17]

Patchset #6 (id:100001) has been deleted

[xdai1, 2016-05-07 22:42:20]

Patchset #6 (id:120001) has been deleted

[xdai1, 2016-05-09 19:38:17]

Patchset #5 (id:80001) has been deleted

[xdai1, 2016-05-09 19:38:23]

Patchset #5 (id:140001) has been deleted

[xdai1, 2016-05-09 19:39:07]

The CQ bit was checked by xdai@chromium.org

[xdai1, 2016-05-09 19:39:07]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org
Link to the patchset: https://codereview.chromium.org/1954853002/#ps160001
(title: "Fix failed browsertest.")

[commit-bot: I haz the power, 2016-05-09 19:39:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1954853002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1954853002/160001

[commit-bot: I haz the power, 2016-05-09 20:30:58]

Committed patchset #5 (id:160001)

[commit-bot: I haz the power, 2016-05-09 20:33:01]

Description was changed from

==========
When user presses Ctrl-N, we should use the current active window's profile to
determine on which profile we should open a new window.

BUG=608840
==========

to

==========
When user presses Ctrl-N, we should use the current active window's profile to
determine on which profile we should open a new window.

BUG=608840

Committed: https://crrev.com/ffeee4dcafd17d3658c3c659cf93aa49dcc90da8
Cr-Commit-Position: refs/heads/master@{#392414}
==========

[commit-bot: I haz the power, 2016-05-09 20:33:02]

Patchset 5 (id:??) landed as
https://crrev.com/ffeee4dcafd17d3658c3c659cf93aa49dcc90da8
Cr-Commit-Position: refs/heads/master@{#392414}