Uber tray unresponsive with Multiprofile enabled

Uber tray unresponsive with Multiprofile enabled

In ExistingUserController::PerformLogin(), There was a missing call to
ExistingUserController::PerformLoginFinishedActions(), which should
re-enables clicking on the status tray after it has been disabled in
ExistingUserController::PerformPreLoginActions().

BUG=427047
TEST=manual

Committed: https://crrev.com/4c09e0ce28ae0ee3a6c54c72e348017d3cda77d8
Cr-Commit-Position: refs/heads/master@{#301975}

[afakhry, 2014-10-28 16:00:10]

afakhry@chromium.org changed reviewers:
+ alemate@chromium.org

[Nikita (slow), 2014-10-28 16:31:18]

nkostylev@chromium.org changed reviewers:
+ nkostylev@chromium.org

[Nikita (slow), 2014-10-28 16:31:19]

Let me try to reproduce this issue.

https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/existing_user_controller.cc:443:
PerformLoginFinishedActions(false);
That's not a correct place for this call since login request has just been
created - previous PerformLogin() call.

[afakhry, 2014-10-28 17:06:30]

https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/existing_user_controller.cc:443:
PerformLoginFinishedActions(false);
On 2014/10/28 16:31:19, Nikita Kostylev wrote:
> That's not a correct place for this call since login request has just been
> created - previous PerformLogin() call.

As per the comments documentations for
ExistingUserController::PerformLoginFinishedActions(), this function performs
set of actions when login has been completed or has been cancelled. However in
this case it never gets called. 

Please let me know where the correct place it should be placed is, in order to
guarantee that it gets called.

[Nikita (slow), 2014-10-28 17:15:23]

https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/existing_user_controller.cc:443:
PerformLoginFinishedActions(false);
On 2014/10/28 17:06:30, afakhry wrote:
> On 2014/10/28 16:31:19, Nikita Kostylev wrote:
> > That's not a correct place for this call since login request has just been
> > created - previous PerformLogin() call.
> 
> As per the comments documentations for
> ExistingUserController::PerformLoginFinishedActions(), this function performs
> set of actions when login has been completed or has been cancelled. However in
> this case it never gets called. 
> 
> Please let me know where the correct place it should be placed is, in order to
> guarantee that it gets called.

I'm trying to verify why this call is not unblocking status area:

void ExistingUserController::OnProfilePrepared(Profile* profile,
                                               bool browser_launched) {
  // Reenable clicking on other windows and status area.
  login_display_->SetUIEnabled(true);
  ...
}

[afakhry, 2014-10-28 18:43:23]

https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/existing_user_controller.cc:443:
PerformLoginFinishedActions(false);
On 2014/10/28 17:15:22, Nikita Kostylev wrote:
> On 2014/10/28 17:06:30, afakhry wrote:
> > On 2014/10/28 16:31:19, Nikita Kostylev wrote:
> > > That's not a correct place for this call since login request has just been
> > > created - previous PerformLogin() call.
> > 
> > As per the comments documentations for
> > ExistingUserController::PerformLoginFinishedActions(), this function
performs
> > set of actions when login has been completed or has been cancelled. However
in
> > this case it never gets called. 
> > 
> > Please let me know where the correct place it should be placed is, in order
to
> > guarantee that it gets called.
> 
> I'm trying to verify why this call is not unblocking status area:
> 
> void ExistingUserController::OnProfilePrepared(Profile* profile,
>                                                bool browser_launched) {
>   // Reenable clicking on other windows and status area.
>   login_display_->SetUIEnabled(true);
>   ...
> }

Because LoginDisplayHostImpl::OnBrowserCreated() calls
LoginDisplayHostImpl::ResetLoginWindowAndView() (which resets 'login_view_' to
NULL) before the call to WebUILoginDisplay::SetUIEnabled() inside
ExistingUserController::OnProfilePrepared() uses the login_view (which is now
null) to unlock the status area.

[Nikita (slow), 2014-10-29 01:57:00]

On 2014/10/28 18:43:23, afakhry wrote:
>
https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
> File chrome/browser/chromeos/login/existing_user_controller.cc (right):
> 
>
https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
> chrome/browser/chromeos/login/existing_user_controller.cc:443:
> PerformLoginFinishedActions(false);
> On 2014/10/28 17:15:22, Nikita Kostylev wrote:
> > On 2014/10/28 17:06:30, afakhry wrote:
> > > On 2014/10/28 16:31:19, Nikita Kostylev wrote:
> > > > That's not a correct place for this call since login request has just
been
> > > > created - previous PerformLogin() call.
> > > 
> > > As per the comments documentations for
> > > ExistingUserController::PerformLoginFinishedActions(), this function
> performs
> > > set of actions when login has been completed or has been cancelled.
However
> in
> > > this case it never gets called. 
> > > 
> > > Please let me know where the correct place it should be placed is, in
order
> to
> > > guarantee that it gets called.
> > 
> > I'm trying to verify why this call is not unblocking status area:
> > 
> > void ExistingUserController::OnProfilePrepared(Profile* profile,
> >                                                bool browser_launched) {
> >   // Reenable clicking on other windows and status area.
> >   login_display_->SetUIEnabled(true);
> >   ...
> > }
> 
> Because LoginDisplayHostImpl::OnBrowserCreated() calls
> LoginDisplayHostImpl::ResetLoginWindowAndView() (which resets 'login_view_' to
> NULL) before the call to WebUILoginDisplay::SetUIEnabled() inside
> ExistingUserController::OnProfilePrepared() uses the login_view (which is now
> null) to unlock the status area.

How about adding login_view->SetStatusAreaVisible(true) just there, in
ResetLoginWindowAndView() ?

[afakhry, 2014-10-29 16:24:49]

On 2014/10/29 01:57:00, Nikita (OOO Oct 29 - Nov 5) wrote:
> On 2014/10/28 18:43:23, afakhry wrote:
> >
>
https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
> > File chrome/browser/chromeos/login/existing_user_controller.cc (right):
> > 
> >
>
https://codereview.chromium.org/682103002/diff/20001/chrome/browser/chromeos/...
> > chrome/browser/chromeos/login/existing_user_controller.cc:443:
> > PerformLoginFinishedActions(false);
> > On 2014/10/28 17:15:22, Nikita Kostylev wrote:
> > > On 2014/10/28 17:06:30, afakhry wrote:
> > > > On 2014/10/28 16:31:19, Nikita Kostylev wrote:
> > > > > That's not a correct place for this call since login request has just
> been
> > > > > created - previous PerformLogin() call.
> > > > 
> > > > As per the comments documentations for
> > > > ExistingUserController::PerformLoginFinishedActions(), this function
> > performs
> > > > set of actions when login has been completed or has been cancelled.
> However
> > in
> > > > this case it never gets called. 
> > > > 
> > > > Please let me know where the correct place it should be placed is, in
> order
> > to
> > > > guarantee that it gets called.
> > > 
> > > I'm trying to verify why this call is not unblocking status area:
> > > 
> > > void ExistingUserController::OnProfilePrepared(Profile* profile,
> > >                                                bool browser_launched) {
> > >   // Reenable clicking on other windows and status area.
> > >   login_display_->SetUIEnabled(true);
> > >   ...
> > > }
> > 
> > Because LoginDisplayHostImpl::OnBrowserCreated() calls
> > LoginDisplayHostImpl::ResetLoginWindowAndView() (which resets 'login_view_'
to
> > NULL) before the call to WebUILoginDisplay::SetUIEnabled() inside
> > ExistingUserController::OnProfilePrepared() uses the login_view (which is
now
> > null) to unlock the status area.
> 
> How about adding login_view->SetStatusAreaVisible(true) just there, in
> ResetLoginWindowAndView() ?

That doesn't fix it since the problem is not that the status area is invisible,
but rather it's disabled. Calling login_view->SetUIEnabled(true) instead. This
fixes it.

[Nikita (slow), 2014-10-29 21:25:43]

lgtm

[afakhry, 2014-10-29 21:38:43]

The CQ bit was checked by afakhry@chromium.org

[commit-bot: I haz the power, 2014-10-29 21:40:11]

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

[commit-bot: I haz the power, 2014-10-29 23:28:28]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-10-29 23:29:21]

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