Make the new Gaia password separated signin flow modal.

Make the new Gaia password separated signin flow modal.

BUG=533004
Committed: https://crrev.com/7a71e2529da7d10316cdacbd2c20697594ae2746
Cr-Commit-Position: refs/heads/master@{#362722}

[anthonyvd, 2015-11-09 21:51:34]

anthonyvd@chromium.org changed reviewers:
+ rogerta@chromium.org

[anthonyvd, 2015-11-09 21:51:34]

Hi Roger,

Can you please take a quick look at this modal-izing of the new Gaia sign in
flow? You're only owner of one of those files but I'd also appreciate your
general wisdom, especially w.r.t naming and code organization.

Thanks a ton :)

[Roger Tawa OOO till Jul 10th, 2015-11-11 19:34:30]

lg, a couple of questions below.

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_view.cc:2535: mode ==
AVATAR_BUBBLE_MODE_REAUTH)) {
Call SigninViewController::ShouldShowModalSigninForMode() instead ?

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/signin_view_controller.cc:128:
ModalSigninDelegate* delegate = new ModalSigninDelegate(
Should you DCHECK modal_signin_widget_ is null?  Or maybe close it if open?

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/signin/inline_login_handler_impl.cc (right):

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:720: }
Nit: don't need { and }

[anthonyvd, 2015-11-24 16:28:04]

Thanks for looking! I'll make sure it builds on all platforms and that tests
pass then loop in owners.

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_view.cc:2535: mode ==
AVATAR_BUBBLE_MODE_REAUTH)) {
On 2015/11/11 19:34:30, Roger Tawa wrote:
> Call SigninViewController::ShouldShowModalSigninForMode() instead ?

Missed that, thanks!

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/profiles/signin_view_controller.cc:128:
ModalSigninDelegate* delegate = new ModalSigninDelegate(
On 2015/11/11 19:34:30, Roger Tawa wrote:
> Should you DCHECK modal_signin_widget_ is null?  Or maybe close it if open?

Good point. Closing it is probably the right thing to do since we don't want
multiple of those per browser yet nothing is preventing this in the UI.

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/signin/inline_login_handler_impl.cc (right):

https://codereview.chromium.org/1413533009/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/signin/inline_login_handler_impl.cc:720: }
On 2015/11/11 19:34:30, Roger Tawa wrote:
> Nit: don't need { and }

Done.

[anthonyvd, 2015-11-24 23:18:17]

anthonyvd@chromium.org changed reviewers:
+ rsesek@chromium.org, sky@chromium.org

[anthonyvd, 2015-11-24 23:18:18]

Hi everyone, can you please take a look at this CL that moves the new Chrome
sign in flow to a tab modal dialog as part of the sign in flow redesign?

This change is still behind the password separated signin flag and this CL only
includes the views implementation, with the mac one to come in a follow up CL.

rsesek@ can you PTAL at the c/b/ui/cocoa changes?
sky@ can you PTAL at the c/b/ui, c/b/ui/views, and c/test files?

Thanks a lot for your time.

[Robert Sesek, 2015-11-24 23:27:34]

cocoa/ LGTM

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/brow...
File chrome/browser/ui/browser_window.h (right):

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/brow...
chrome/browser/ui/browser_window.h:390: virtual void
ShowModalSigninWindow(AvatarBubbleMode mode) = 0;
This should have a comment, as most of the other methods on this class do.

[sky, 2015-11-25 00:30:07]

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:68: return
views::DialogDelegate::CreateDialogFrameView(widget);
Is there a reason you're not subclassing DialogDelegateView?

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:73: };
DISALLOW...

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:82: GURL url;
The code for determining the url doesn't seem platform specific. Can it be some
where general?

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:138:
modal_signin_widget_ = nullptr;
What if the modal_signin_widget_ is deleted out from under you?

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.h (right):

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.h:17: };
nit: no ;

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.h:22: static
views::WebView* CreateGaiaWebView(WebContentsDelegate* delegate,
somewhitespacewouldbenice.
And some comments too.

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.h:30: views::Widget*
modal_signin_widget_;  // Not owned.
Should this be closed if the destructor is hit and it's non-null?

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.h:31: };
DISALLOW...

[anthonyvd, 2015-11-25 18:49:31]

Thank you very much for your feedback, here's a new patchset that should address
those comments.

rogerta@, I touched some more signin files in the latest patchset, can you
please take another look?

Thanks!

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/brow...
File chrome/browser/ui/browser_window.h (right):

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/brow...
chrome/browser/ui/browser_window.h:390: virtual void
ShowModalSigninWindow(AvatarBubbleMode mode) = 0;
On 2015/11/24 23:27:34, Robert Sesek wrote:
> This should have a comment, as most of the other methods on this class do.

Done.

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:68: return
views::DialogDelegate::CreateDialogFrameView(widget);
On 2015/11/25 00:30:07, sky wrote:
> Is there a reason you're not subclassing DialogDelegateView?

Doing it that way automatically adds the OK/Cancel buttons to the constrained
window. Am I doing something wrong?

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:73: };
On 2015/11/25 00:30:07, sky wrote:
> DISALLOW...

Done.

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:82: GURL url;
On 2015/11/25 00:30:07, sky wrote:
> The code for determining the url doesn't seem platform specific. Can it be
some
> where general?

Done.

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:138:
modal_signin_widget_ = nullptr;
On 2015/11/25 00:30:07, sky wrote:
> What if the modal_signin_widget_ is deleted out from under you?

Good point. This latest patchset fixes this by setting it to nullptr when the
delegate is deleted. The pointer to implementation shenanigans were meant to
keep the Reset() function out of SigninViewController's public interface. Let me
know what you think.

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.h (right):

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.h:17: };
On 2015/11/25 00:30:07, sky wrote:
> nit: no ;

Done.

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.h:22: static
views::WebView* CreateGaiaWebView(WebContentsDelegate* delegate,
On 2015/11/25 00:30:07, sky wrote:
> somewhitespacewouldbenice.
> And some comments too.

Done.

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.h:30: views::Widget*
modal_signin_widget_;  // Not owned.
On 2015/11/25 00:30:07, sky wrote:
> Should this be closed if the destructor is hit and it's non-null?

Done.

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.h:31: };
On 2015/11/25 00:30:07, sky wrote:
> DISALLOW...

Done.

[Roger Tawa OOO till Jul 10th, 2015-11-25 19:04:21]

c/b/s/ lgtm

[sky, 2015-11-25 20:57:17]

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:68: return
views::DialogDelegate::CreateDialogFrameView(widget);
On 2015/11/25 18:49:31, anthonyvd wrote:
> On 2015/11/25 00:30:07, sky wrote:
> > Is there a reason you're not subclassing DialogDelegateView?
> 
> Doing it that way automatically adds the OK/Cancel buttons to the constrained
> window. Am I doing something wrong?

You can override GetDialogButtons if you don't want them.

[anthonyvd, 2015-11-25 23:05:22]

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/120001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:68: return
views::DialogDelegate::CreateDialogFrameView(widget);
On 2015/11/25 20:57:16, sky wrote:
> On 2015/11/25 18:49:31, anthonyvd wrote:
> > On 2015/11/25 00:30:07, sky wrote:
> > > Is there a reason you're not subclassing DialogDelegateView?
> > 
> > Doing it that way automatically adds the OK/Cancel buttons to the
constrained
> > window. Am I doing something wrong?
> 
> You can override GetDialogButtons if you don't want them.

Neat, thanks for the info!

Done.

[sky, 2015-11-30 16:16:39]

https://codereview.chromium.org/1413533009/diff/160001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:38:
modal_signin_widget_->Close();
Close is not synchronous. DeleteDelegate will be called async, and this code
doesn't handle that well.

[anthonyvd, 2015-11-30 21:59:34]

Thanks for the feedback, here's a new patchset that I think addresses it!

https://codereview.chromium.org/1413533009/diff/160001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/160001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:38:
modal_signin_widget_->Close();
On 2015/11/30 16:16:38, sky wrote:
> Close is not synchronous. DeleteDelegate will be called async, and this code
> doesn't handle that well.

Ahh, I didn't think about that!

I believe the latest patchset addresses this by having the delegates observe the
view controller, which in turn only notifies observers when the dialog should be
closed. Ultimately, the delegates register/unregister themselves from the event
and are the only ones responsible for closing the window and deleting
themselves.

Let me know what you think.

[sky, 2015-11-30 23:54:23]

https://codereview.chromium.org/1413533009/diff/180001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/180001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:29: class
SigninViewControllerImpl {
Why do you need this implementation class? Can't this functionality be part of
SigninViewController? By having an extra class (with no documentation) you make
it harder to understand.

https://codereview.chromium.org/1413533009/diff/180001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:53: explicit
ModalSigninDelegate(SigninViewControllerImpl* signin_view_controller,
no explicit.

https://codereview.chromium.org/1413533009/diff/180001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:64: views::View*
GetContentsView() override {
Prefix and group sections that are implementations of interfaces. As you have it
now it's hard to tell what is what.

[anthonyvd, 2015-12-01 16:37:08]

https://codereview.chromium.org/1413533009/diff/180001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/180001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:29: class
SigninViewControllerImpl {
On 2015/11/30 23:54:23, sky wrote:
> Why do you need this implementation class? Can't this functionality be part of
> SigninViewController? By having an extra class (with no documentation) you
make
> it harder to understand.

I wasn't too sure about leaving it in because I liked the fact that it was
hiding the whole Observer dance from the public interface. You bring a good
point though, the added complexity doesn't seem very worthwhile.

Done.

https://codereview.chromium.org/1413533009/diff/180001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:53: explicit
ModalSigninDelegate(SigninViewControllerImpl* signin_view_controller,
On 2015/11/30 23:54:23, sky wrote:
> no explicit.

Done.

https://codereview.chromium.org/1413533009/diff/180001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:64: views::View*
GetContentsView() override {
On 2015/11/30 23:54:23, sky wrote:
> Prefix and group sections that are implementations of interfaces. As you have
it
> now it's hard to tell what is what.

Done.

[sky, 2015-12-01 21:12:37]

https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:64:
signin_view_controller_->RemoveObserver(this);
Again, won't this crash if SigninViewController has since been destroyed?

There is a 1:1 relation between SigninViewController and ModalSigninDelegate.
ModalSigninDelegate may outlive SigninViewController. I don't think this
warrants an observer. I recommend the following:
. SigninViewController having a member for ModalSigninDelegate, I'll call it
modal_signin_delegate_;
. SigninViewController::CloseModalSignin() calls
modal_signin_delegate_->CloseModalSignin().
. ModalSigninDelegate::CloseModalSignin() nulls out signin_view_controller_.
. ModalSigninDelegate::DeleteDelegate() calls to signin_view_controller_ (if
non-null) that it is no longer valid.
. ModalSigninDelegate::OnCloseRequested() needs to also notify
signin_view_controller_ it's going away.

https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.h (right):

https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.h:20: class
SigninViewController : public content::WebContentsDelegate {
Why do you need this and ModalSigninDelegate to be WebContentsDelegate? AFAICT
you aren't overriding anything from WebContentsDelegate.

[anthonyvd, 2015-12-01 22:25:04]

Thanks for the advice, the changes discussed in comments are implemented over
the 2 latest patchsets.

https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:64:
signin_view_controller_->RemoveObserver(this);
On 2015/12/01 21:12:37, sky wrote:
> Again, won't this crash if SigninViewController has since been destroyed?

You're right, I was focusing on the other way around where DeleteDelegate is
called when SigninViewController never got a call to close the modal and didn't
think about this scenario.

> 
> There is a 1:1 relation between SigninViewController and ModalSigninDelegate.
> ModalSigninDelegate may outlive SigninViewController. I don't think this
> warrants an observer. I recommend the following:
> . SigninViewController having a member for ModalSigninDelegate, I'll call it
> modal_signin_delegate_;
> . SigninViewController::CloseModalSignin() calls
> modal_signin_delegate_->CloseModalSignin().
> . ModalSigninDelegate::CloseModalSignin() nulls out signin_view_controller_.
> . ModalSigninDelegate::DeleteDelegate() calls to signin_view_controller_ (if
> non-null) that it is no longer valid.
> . ModalSigninDelegate::OnCloseRequested() needs to also notify
> signin_view_controller_ it's going away.

Thanks a ton for the advice. This new patchset should implement it this way.
Sorry for the back-and-forth.

https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.h (right):

https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.h:20: class
SigninViewController : public content::WebContentsDelegate {
On 2015/12/01 21:12:37, sky wrote:
> Why do you need this and ModalSigninDelegate to be WebContentsDelegate? AFAICT
> you aren't overriding anything from WebContentsDelegate.

I don't believe I need it, it's some leftover from when I was moving
CreateGaiaWebView from ProfileChooserController and thought this class would do
more things. Thanks for noticing :)

[anthonyvd, 2015-12-01 22:26:20]

On 2015/12/01 22:25:04, anthonyvd wrote:
> Thanks for the advice, the changes discussed in comments are implemented over
> the 2 latest patchsets.
> 
>
https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
> File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):
> 
>
https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
> chrome/browser/ui/views/profiles/signin_view_controller.cc:64:
> signin_view_controller_->RemoveObserver(this);
> On 2015/12/01 21:12:37, sky wrote:
> > Again, won't this crash if SigninViewController has since been destroyed?
> 
> You're right, I was focusing on the other way around where DeleteDelegate is
> called when SigninViewController never got a call to close the modal and
didn't
> think about this scenario.
> 
> > 
> > There is a 1:1 relation between SigninViewController and
ModalSigninDelegate.
> > ModalSigninDelegate may outlive SigninViewController. I don't think this
> > warrants an observer. I recommend the following:
> > . SigninViewController having a member for ModalSigninDelegate, I'll call it
> > modal_signin_delegate_;
> > . SigninViewController::CloseModalSignin() calls
> > modal_signin_delegate_->CloseModalSignin().
> > . ModalSigninDelegate::CloseModalSignin() nulls out signin_view_controller_.
> > . ModalSigninDelegate::DeleteDelegate() calls to signin_view_controller_ (if
> > non-null) that it is no longer valid.
> > . ModalSigninDelegate::OnCloseRequested() needs to also notify
> > signin_view_controller_ it's going away.
> 
> Thanks a ton for the advice. This new patchset should implement it this way.
> Sorry for the back-and-forth.
> 
>
https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
> File chrome/browser/ui/views/profiles/signin_view_controller.h (right):
> 
>
https://codereview.chromium.org/1413533009/diff/200001/chrome/browser/ui/view...
> chrome/browser/ui/views/profiles/signin_view_controller.h:20: class
> SigninViewController : public content::WebContentsDelegate {
> On 2015/12/01 21:12:37, sky wrote:
> > Why do you need this and ModalSigninDelegate to be WebContentsDelegate?
AFAICT
> > you aren't overriding anything from WebContentsDelegate.
> 
> I don't believe I need it, it's some leftover from when I was moving
> CreateGaiaWebView from ProfileChooserController and thought this class would
do
> more things. Thanks for noticing :)

Unfortunately, it seems like there's some rebase stuff in patchset 11. The only
relevant changes should be in signin_view_controller.*.

Sorry about that.

[sky, 2015-12-01 23:34:52]

LGTM

https://codereview.chromium.org/1413533009/diff/240001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/240001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:134:
modal_signin_delegate_->CloseModalSignin();
null out modal_signin_delegate_ here, or a DCHECK that after the call
modal_signin_delegate_ is null. I'm fine with either.

[anthonyvd, 2015-12-02 15:24:41]

https://codereview.chromium.org/1413533009/diff/240001/chrome/browser/ui/view...
File chrome/browser/ui/views/profiles/signin_view_controller.cc (right):

https://codereview.chromium.org/1413533009/diff/240001/chrome/browser/ui/view...
chrome/browser/ui/views/profiles/signin_view_controller.cc:134:
modal_signin_delegate_->CloseModalSignin();
On 2015/12/01 23:34:52, sky wrote:
> null out modal_signin_delegate_ here, or a DCHECK that after the call
> modal_signin_delegate_ is null. I'm fine with either.

Done.

[anthonyvd, 2015-12-02 15:24:47]

The CQ bit was checked by anthonyvd@chromium.org

[anthonyvd, 2015-12-02 15:24:47]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
rogerta@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1413533009/#ps260001
(title: "Add DCHECK")

[commit-bot: I haz the power, 2015-12-02 15:25:30]

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

[commit-bot: I haz the power, 2015-12-02 16:06:13]

Committed patchset #14 (id:260001)

[commit-bot: I haz the power, 2015-12-02 16:06:53]

Description was changed from

==========
Make the new Gaia password separated signin flow modal.

BUG=533004
==========

to

==========
Make the new Gaia password separated signin flow modal.

BUG=533004

Committed: https://crrev.com/7a71e2529da7d10316cdacbd2c20697594ae2746
Cr-Commit-Position: refs/heads/master@{#362722}
==========

[commit-bot: I haz the power, 2015-12-02 16:06:54]

Patchset 14 (id:??) landed as
https://crrev.com/7a71e2529da7d10316cdacbd2c20697594ae2746
Cr-Commit-Position: refs/heads/master@{#362722}