Added keyboard shortcut on GAIA "Add User" screen to dogfood GAIA /EmbeddedSignin

Added keyboard shortcut on GAIA "Add User" screen to dogfood GAIA /EmbeddedSignin

BUG=420438
Committed: https://crrev.com/ef2725f7cb299cb4ea945c30e22d35285962d592
Cr-Commit-Position: refs/heads/master@{#299113}

[Roman Sorokin (ftl), 2014-10-10 09:46:29]

rsorokin@chromium.org changed reviewers:
+ ygorshenin@chromium.org

[Roman Sorokin (ftl), 2014-10-10 09:46:53]

Please review

[Nikita (slow), 2014-10-10 10:56:28]

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

[Nikita (slow), 2014-10-10 10:56:28]

https://codereview.chromium.org/645003002/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/645003002/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:249:
embedded_signin_enabled_by_shortcut_ = false;
I suggest clearing this flag only when user explicitly exits GAIA screen because
in case of network error auth extension might get reloaded and it will load
existing flow.

Please add Pavel to this review to make sure that embedded_signin flag is not
checked somewhere later during user sign in i.e. to confirm that   this flag is
only used when loading auth extension.

[Roman Sorokin (ftl), 2014-10-10 11:41:16]

rsorokin@chromium.org changed reviewers:
+ dzhioev@chromium.org

[Roman Sorokin (ftl), 2014-10-10 12:19:32]

https://codereview.chromium.org/645003002/diff/1/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/645003002/diff/1/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:249:
embedded_signin_enabled_by_shortcut_ = false;
On 2014/10/10 10:56:28, Nikita Kostylev wrote:
> I suggest clearing this flag only when user explicitly exits GAIA screen
because
> in case of network error auth extension might get reloaded and it will load
> existing flow.
> 
> Please add Pavel to this review to make sure that embedded_signin flag is not
> checked somewhere later during user sign in i.e. to confirm that   this flag
is
> only used when loading auth extension.

Done.

[ygorshenin1, 2014-10-10 13:25:04]

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:210:
embedded_signin_enabled_by_shortcut_)
I'd prefer to add a field named, say, embedded_signin_enabled to the GaiaContext
and set it up in the LoadAuthExtension() method. So, LoadGaia() method will
still depend on the GaiaContext(), not on the GaiaScreenHandler internal state.

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:211:
params.SetBoolean("useEmbedded", true);
nit: add curly braces around branch.

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:248:
LoadAuthExtension(true, true, false);
Could you please add short comments with arg names after each arg? I mean,
LoadAuthExtension(true /* force */, true /* silent_load */, false /* offline */)
looks more clear.

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.h (right):

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.h:80: // Cancel
embedded signin for the next load.
nit: a blank line before comment.

[Roman Sorokin (ftl), 2014-10-10 13:51:03]

Please review

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc (right):

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:210:
embedded_signin_enabled_by_shortcut_)
On 2014/10/10 13:25:04, ygorshenin1 wrote:
> I'd prefer to add a field named, say, embedded_signin_enabled to the
GaiaContext
> and set it up in the LoadAuthExtension() method. So, LoadGaia() method will
> still depend on the GaiaContext(), not on the GaiaScreenHandler internal
state.

Done.

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:211:
params.SetBoolean("useEmbedded", true);
On 2014/10/10 13:25:04, ygorshenin1 wrote:
> nit: add curly braces around branch.

Acknowledged.

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc:248:
LoadAuthExtension(true, true, false);
On 2014/10/10 13:25:04, ygorshenin1 wrote:
> Could you please add short comments with arg names after each arg? I mean,
> LoadAuthExtension(true /* force */, true /* silent_load */, false /* offline
*/)
> looks more clear.

Done.

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.h (right):

https://codereview.chromium.org/645003002/diff/150001/chrome/browser/ui/webui...
chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.h:80: // Cancel
embedded signin for the next load.
On 2014/10/10 13:25:04, ygorshenin1 wrote:
> nit: a blank line before comment.

Done.

[ygorshenin1, 2014-10-10 14:06:29]

lgtm

[Roman Sorokin (ftl), 2014-10-10 14:08:56]

The CQ bit was checked by rsorokin@chromium.org

[commit-bot: I haz the power, 2014-10-10 14:09:30]

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

[commit-bot: I haz the power, 2014-10-10 14:53:22]

Committed patchset #3 (id:220001)

[commit-bot: I haz the power, 2014-10-10 14:54:05]

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

[Roman Sorokin (ftl), 2014-10-10 15:45:31]

rsorokin@chromium.org changed reviewers:
- dzhioev@chromium.org, nkostylev@chromium.org