[cros,login] Pre-load login extension in the background.

[cros,login] Pre-load login extension in the background.

BUG=chromium-os:3245
TEST=on login screen; click on 'add new user' then click 'cancel'; repeat 3 times; first click should summon gaia page instantly; 2nd and 3rd clicks should show page after a small delay (spinner is shown).

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=109021

[altimofeev, 2011-11-03 14:42:11]

PTAL. Note, I've omitted cookies clearing before pre-loading, since it seems
that they can't contain any harmful information.

[xiyuan, 2011-11-03 16:52:33]

http://codereview.chromium.org/8372093/diff/1/chrome/browser/resources/chrome...
File chrome/browser/resources/chromeos/login/screen_gaia_signin.js (right):

http://codereview.chromium.org/8372093/diff/1/chrome/browser/resources/chrome...
chrome/browser/resources/chromeos/login/screen_gaia_signin.js:139: } else if
(!this.loading) {
When will the two "else" branch below get hit?

It seems a bit strange to me that a SetExtensionUrl call triggers
"loginWebuiRead".

[altimofeev, 2011-11-03 17:22:31]

http://codereview.chromium.org/8372093/diff/1/chrome/browser/resources/chrome...
File chrome/browser/resources/chromeos/login/screen_gaia_signin.js (right):

http://codereview.chromium.org/8372093/diff/1/chrome/browser/resources/chrome...
chrome/browser/resources/chromeos/login/screen_gaia_signin.js:139: } else if
(!this.loading) {
On 2011/11/03 16:52:33, xiyuan wrote:
> When will the two "else" branch below get hit?
> 
> It seems a bit strange to me that a SetExtensionUrl call triggers
> "loginWebuiRead".

* "!this.loading": first click on 'Add new user' button, after extension has
been loaded in background.
* "this.loading": first click on 'Add new user' button, loading is in progress
(probably, network error). So we try to load again.

Do you think I should change SetExtensionUrl to something which implicitly says
that this URL will also be fetched? What about 'StartExtensionLoading'?

[xiyuan, 2011-11-03 17:36:04]

http://codereview.chromium.org/8372093/diff/1/chrome/browser/resources/chrome...
File chrome/browser/resources/chromeos/login/screen_gaia_signin.js (right):

http://codereview.chromium.org/8372093/diff/1/chrome/browser/resources/chrome...
chrome/browser/resources/chromeos/login/screen_gaia_signin.js:139: } else if
(!this.loading) {
On 2011/11/03 17:22:32, altimofeev wrote:
> On 2011/11/03 16:52:33, xiyuan wrote:
> > When will the two "else" branch below get hit?
> > 
> > It seems a bit strange to me that a SetExtensionUrl call triggers
> > "loginWebuiRead".
> 
> * "!this.loading": first click on 'Add new user' button, after extension has
> been loaded in background.

In this case, if Gaia is loaded successfully, "loginWebuiRead" should be fired
already in onMessage handler and we don't need to fire it again here.

> * "this.loading": first click on 'Add new user' button, loading is in progress
> (probably, network error). So we try to load again.
> 

Okay. We probably need this to show proper error message.

> Do you think I should change SetExtensionUrl to something which implicitly
says
> that this URL will also be fetched? What about 'StartExtensionLoading'?

Think a name implies loading is better. I prefer to call it "LoadExtension" or
"LoadAuthExt" etc.

[altimofeev, 2011-11-07 16:21:59]

Thank you for the comments. PTAL.

http://codereview.chromium.org/8372093/diff/1/chrome/browser/resources/chrome...
File chrome/browser/resources/chromeos/login/screen_gaia_signin.js (right):

http://codereview.chromium.org/8372093/diff/1/chrome/browser/resources/chrome...
chrome/browser/resources/chromeos/login/screen_gaia_signin.js:139: } else if
(!this.loading) {
On 2011/11/03 17:36:04, xiyuan wrote:
> On 2011/11/03 17:22:32, altimofeev wrote:
> > On 2011/11/03 16:52:33, xiyuan wrote:
> > > When will the two "else" branch below get hit?
> > > 
> > > It seems a bit strange to me that a SetExtensionUrl call triggers
> > > "loginWebuiRead".
> > 
> > * "!this.loading": first click on 'Add new user' button, after extension has
> > been loaded in background.
> 
> In this case, if Gaia is loaded successfully, "loginWebuiRead" should be fired
> already in onMessage handler and we don't need to fire it again here.

Done - moved the logic fully to the C++ part.

> > * "this.loading": first click on 'Add new user' button, loading is in
progress
> > (probably, network error). So we try to load again.
> > 
> 
> Okay. We probably need this to show proper error message.
> 
> > Do you think I should change SetExtensionUrl to something which implicitly
> says
> > that this URL will also be fetched? What about 'StartExtensionLoading'?
> 
> Think a name implies loading is better. I prefer to call it "LoadExtension" or
> "LoadAuthExt" etc.

Done.

[xiyuan, 2011-11-07 17:46:34]

LGTM

[Nikita (slow), 2011-11-08 09:13:53]

lgtm

http://codereview.chromium.org/8372093/diff/7001/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/login/screen_account_picker.js (right):

http://codereview.chromium.org/8372093/diff/7001/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/login/screen_account_picker.js:47:
chrome.send('accountPickerReady', []);
As we figured out animation is currently turned off.

[altimofeev, 2011-11-08 11:40:20]

http://codereview.chromium.org/8372093/diff/7001/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/login/screen_account_picker.js (right):

http://codereview.chromium.org/8372093/diff/7001/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/login/screen_account_picker.js:47:
chrome.send('accountPickerReady', []);
On 2011/11/08 09:13:53, Nikita Kostylev wrote:
> As we figured out animation is currently turned off.

Done.

[commit-bot: I haz the power, 2011-11-08 11:43:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/altimofeev@chromium.org/8372093/13002

[commit-bot: I haz the power, 2011-11-08 13:05:55]

Try job failure for 8372093-13002 (retry) on win_rel for step
"chrome_frame_net_tests".
It's a second try, previously, step "chrome_frame_net_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...