Wire up the Chrome Sign In modal dialog from the password bubble promo.

Wire up the Chrome Sign In modal dialog from the password bubble promo.

The promo is still disabled as the Finch config isn't created.

BUG=615825
Committed: https://crrev.com/38c1ed36924843e5c617f5f36f635fe29d7f1e89
Cr-Commit-Position: refs/heads/master@{#398881}

[vasilii, 2016-06-08 11:49:11]

Description was changed from

==========
Wire up the Chrome Sign In modal dialog from the password bubble promo.

BUG=615825
==========

to

==========
Wire up the Chrome Sign In modal dialog from the password bubble promo.

The promo is still disabled as the Finch config isn't created.

BUG=615825
==========

[vasilii, 2016-06-08 11:59:34]

vasilii@chromium.org changed reviewers:
+ gogerald@chromium.org

[vasilii, 2016-06-08 12:18:49]

vasilii@chromium.org changed reviewers:
+ isherman@chromium.org, rogerta@chromium.org

[vasilii, 2016-06-08 12:22:01]

Hi guys,

please review the CL.

[gogerald1, 2016-06-08 14:57:19]

https://codereview.chromium.org/2049683002/diff/1/chrome/browser/ui/passwords...
File chrome/browser/ui/passwords/manage_passwords_ui_controller.cc (right):

https://codereview.chromium.org/2049683002/diff/1/chrome/browser/ui/passwords...
chrome/browser/ui/passwords/manage_passwords_ui_controller.cc:337:
browser->ShowModalSigninWindow(
May use 'void BrowserView::ShowAvatarBubbleFromAvatarButton' instead, new sign
in flow hasn't been completely rolled out.

https://codereview.chromium.org/2049683002/diff/1/components/signin/core/brow...
File components/signin/core/browser/signin_metrics.h (right):

https://codereview.chromium.org/2049683002/diff/1/components/signin/core/brow...
components/signin/core/browser/signin_metrics.h:139:
ACCESS_POINT_PASSWORD_BUBBLE,
move it after ACCESS_POINT_UNKNOWN, and update SigninAccessPoint in
histograms.xml

[vasilii, 2016-06-08 17:48:25]

https://codereview.chromium.org/2049683002/diff/1/chrome/browser/ui/passwords...
File chrome/browser/ui/passwords/manage_passwords_ui_controller.cc (right):

https://codereview.chromium.org/2049683002/diff/1/chrome/browser/ui/passwords...
chrome/browser/ui/passwords/manage_passwords_ui_controller.cc:337:
browser->ShowModalSigninWindow(
On 2016/06/08 14:57:19, gogerald1 wrote:
> May use 'void BrowserView::ShowAvatarBubbleFromAvatarButton' instead, new sign
> in flow hasn't been completely rolled out.

Done.

https://codereview.chromium.org/2049683002/diff/1/components/signin/core/brow...
File components/signin/core/browser/signin_metrics.h (right):

https://codereview.chromium.org/2049683002/diff/1/components/signin/core/brow...
components/signin/core/browser/signin_metrics.h:139:
ACCESS_POINT_PASSWORD_BUBBLE,
On 2016/06/08 14:57:19, gogerald1 wrote:
> move it after ACCESS_POINT_UNKNOWN, and update SigninAccessPoint in
> histograms.xml

Done.

[gogerald1, 2016-06-08 18:02:56]

LGTM with a nit,

https://codereview.chromium.org/2049683002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/signin/inline_login_handler.cc (right):

https://codereview.chromium.org/2049683002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/signin/inline_login_handler.cc:168: case
signin_metrics::AccessPoint::ACCESS_POINT_PASSWORD_BUBBLE:
nit, better to keep the same order, move it below ACCESS_POINT_UNKOWN

[Ilya Sherman, 2016-06-08 22:32:15]

metrics lgtm

[vasilii, 2016-06-09 08:37:03]

Roger, please review.

https://codereview.chromium.org/2049683002/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/signin/inline_login_handler.cc (right):

https://codereview.chromium.org/2049683002/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/signin/inline_login_handler.cc:168: case
signin_metrics::AccessPoint::ACCESS_POINT_PASSWORD_BUBBLE:
On 2016/06/08 18:02:56, gogerald1 wrote:
> nit, better to keep the same order, move it below ACCESS_POINT_UNKOWN

Done.

[Roger Tawa OOO till Jul 10th, 2016-06-09 14:23:26]

lgtm

[vasilii, 2016-06-09 14:56:49]

The CQ bit was checked by vasilii@chromium.org

[vasilii, 2016-06-09 14:56:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, gogerald@chromium.org
Link to the patchset: https://codereview.chromium.org/2049683002/#ps40001
(title: "nit")

[commit-bot: I haz the power, 2016-06-09 14:57:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2049683002/40001

[commit-bot: I haz the power, 2016-06-09 15:04:30]

Description was changed from

==========
Wire up the Chrome Sign In modal dialog from the password bubble promo.

The promo is still disabled as the Finch config isn't created.

BUG=615825
==========

to

==========
Wire up the Chrome Sign In modal dialog from the password bubble promo.

The promo is still disabled as the Finch config isn't created.

BUG=615825
==========

[commit-bot: I haz the power, 2016-06-09 15:04:32]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-09 15:05:59]

Description was changed from

==========
Wire up the Chrome Sign In modal dialog from the password bubble promo.

The promo is still disabled as the Finch config isn't created.

BUG=615825
==========

to

==========
Wire up the Chrome Sign In modal dialog from the password bubble promo.

The promo is still disabled as the Finch config isn't created.

BUG=615825

Committed: https://crrev.com/38c1ed36924843e5c617f5f36f635fe29d7f1e89
Cr-Commit-Position: refs/heads/master@{#398881}
==========

[commit-bot: I haz the power, 2016-06-09 15:06:00]

Patchset 3 (id:??) landed as
https://crrev.com/38c1ed36924843e5c617f5f36f635fe29d7f1e89
Cr-Commit-Position: refs/heads/master@{#398881}