[cros,webui] Captive portal on login screen proper handling.

[cros,webui] Captive portal on login screen proper handling.

Changed login screen to observe network changes directly, instead of 'navigator.onLine' usage.
This CL also fixed the problem captive portal wasn't being hide, when good connection had appeared.

BUG=chromium-os:18744
TEST=manual

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

[xiyuan, 2011-09-26 17:06:13]

LGTM

http://codereview.chromium.org/8043024/diff/3001/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

http://codereview.chromium.org/8043024/diff/3001/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:560:
network_state_informer_.reset(new NetworkStateInformer(web_ui_));
nit: This should not happen since Initialize should be called before all the
handlers. If we need to do this, could we put it in a EnsureNetworkStateInformer
function?

[altimofeev, 2011-09-26 18:11:41]

Thanks for the comment!

http://codereview.chromium.org/8043024/diff/3001/chrome/browser/ui/webui/chro...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

http://codereview.chromium.org/8043024/diff/3001/chrome/browser/ui/webui/chro...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:560:
network_state_informer_.reset(new NetworkStateInformer(web_ui_));
On 2011/09/26 17:06:14, xiyuan wrote:
> nit: This should not happen since Initialize should be called before all the
> handlers. If we need to do this, could we put it in a
EnsureNetworkStateInformer
> function?

Initialize() is called after JS was loaded, but JS uses
'loginAddNetworkObserver' while loading and I don't see any crime in it. So I
have moved initialization of |network_state_informer_| to RegisterMessages(),
since |network_state_informer_| is used by the handlers.

[xiyuan, 2011-09-26 18:17:13]

still LGTM.

On 2011/09/26 18:11:41, altimofeev wrote:
> Thanks for the comment!
> 
>
http://codereview.chromium.org/8043024/diff/3001/chrome/browser/ui/webui/chro...
> File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):
> 
>
http://codereview.chromium.org/8043024/diff/3001/chrome/browser/ui/webui/chro...
> chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:560:
> network_state_informer_.reset(new NetworkStateInformer(web_ui_));
> On 2011/09/26 17:06:14, xiyuan wrote:
> > nit: This should not happen since Initialize should be called before all the
> > handlers. If we need to do this, could we put it in a
> EnsureNetworkStateInformer
> > function?
> 
> Initialize() is called after JS was loaded, but JS uses
> 'loginAddNetworkObserver' while loading and I don't see any crime in it. So I
> have moved initialization of |network_state_informer_| to RegisterMessages(),
> since |network_state_informer_| is used by the handlers.

I forget that decorate is called before Initialize. So you are right the
loginAddNetworkObserver is called without network_state_informer_. 

Your new way is better. :)

[xiyuan, 2011-09-26 21:24:38]

With this change, could we simplify OfflineMessageScreen.onFrameError to just
call update()?

[altimofeev, 2011-09-27 13:36:16]

On 2011/09/26 21:24:38, xiyuan wrote:
> With this change, could we simplify OfflineMessageScreen.onFrameError to just
> call update()?

Done.