NetworkPortalDetector/NetworkStateInformer: Switch over to use NetworkStateHandler

NetworkPortalDetector/NetworkStateInformer: Switch over to use NetworkStateHandler

This removes the direct network library dependency for NetworkPortalDetector,
NetworkStateInformer, and SigninScreenHandler. UpdateScreen and
LocallyManagedUserCreationHandler no longer depend on NetworkLibrary as well.
   
Also change active -> default network in these to be consistent with
terminology we use in NetworkStateHandler.
   
BUG=189093, 189009
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=200968

[gauravsh, 2013-05-02 00:24:25]

nkostylev: Please review portal detector (c/b/chromeos/net/*) changes, as well
as everything under c/b/chromeos/login and c/b/ui/webui/chromeos/login/*.

[gauravsh, 2013-05-02 19:04:22]

pneubeck: please review changes to NetworkState and the proxy config.

[Nikita (slow), 2013-05-02 19:49:36]

Yuri is the better reviewer than me for this CL.

+Yuri

[pneubeck (no reviews), 2013-05-02 22:11:13]

proxy_config* and network_state*
lgtm after the other comments are addressed.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/screens/update_screen_browsertest.cc (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/screens/update_screen_browsertest.cc:150:
scoped_ptr<NetworkState> stub_ethernet_;
scoped_ptr is not necessary, you can use normal members.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
(right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:69:
NetworkStateHandler::Get()->InitShillPropertyHandler();
This last line shouldn't be necessary, if you instead swap the order of the
first two statements.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:76:
ethernet_network_ =
References to NetworkState should never be stored, not even in unit tests. 
Please use only the service paths as identifier and refetch the state where
necessary.

[ygorshenin1, 2013-05-06 09:14:39]

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
File
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.cc
(right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.cc:7:
#include "base/values.h"
nit: not needed there.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
File
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h
(right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h:79:
// ConnectivityStateHelperObserver implementation:
Wrong comment, could you please fix it?

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h:81:
const NetworkState* network,
Forward declaration for NetworkState is needed.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_impl.cc (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:103: state_ =
STATE_IDLE;
Please, leave DCHECK(NetworkStateHandler::Get()) here, or, if it's guaranteed
that it's always non-NULL, remove check at line #116.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:204: if
(network_changed) {
You can omit curly braces here.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_impl.h (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.h:170: std::string
default_connection_state_;
Please, add comment to the |connection_state_|.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.h:190: // Number of
portal detection attemps for an default network.
s/an/a

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
(right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:27: void
ErrorCallbackFunction(
Wrong indentation, empty line needed after line #26.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:66: virtual
void SetupNetworkStateHandler() {
Why this method is virtual?

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:196:
base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction)
Please, combine lines #196 and #197.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/login/network_state_informer.h (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/login/network_state_informer.h:10: 
#include "base/compiler_specific.h"

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:649:
params.SetString("lastNetworkType", connection_type);
As you're passing connection_type as string instead of integer, please, fix 

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

and

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

[pneubeck (no reviews), 2013-05-06 09:43:18]

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
File
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.cc
(right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.cc:7:
#include "base/values.h"
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> nit: not needed there.
line 218 :-)

[ygorshenin1, 2013-05-06 09:45:47]

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
File
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.cc
(right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.cc:7:
#include "base/values.h"
I see, sorry :)

On 2013/05/06 09:43:18, pneubeck wrote:
> On 2013/05/06 09:14:39, ygorshenin1 wrote:
> > nit: not needed there.
> line 218 :-)

[gauravsh, 2013-05-07 23:38:09]

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
File
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h
(right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h:79:
// ConnectivityStateHelperObserver implementation:
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> Wrong comment, could you please fix it?

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/managed/locally_managed_user_creation_screen.h:81:
const NetworkState* network,
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> Forward declaration for NetworkState is needed.

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/screens/update_screen_browsertest.cc (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/screens/update_screen_browsertest.cc:150:
scoped_ptr<NetworkState> stub_ethernet_;
On 2013/05/02 22:11:13, pneubeck wrote:
> scoped_ptr is not necessary, you can use normal members.

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_impl.cc (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:103: state_ =
STATE_IDLE;
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> Please, leave DCHECK(NetworkStateHandler::Get()) here, or, if it's guaranteed
> that it's always non-NULL, remove check at line #116.

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:204: if
(network_changed) {
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> You can omit curly braces here.

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_impl.h (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.h:170: std::string
default_connection_state_;
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> Please, add comment to the |connection_state_|.

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.h:190: // Number of
portal detection attemps for an default network.
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> s/an/a

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc
(right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:27: void
ErrorCallbackFunction(
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> Wrong indentation, empty line needed after line #26.

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:66: virtual
void SetupNetworkStateHandler() {
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> Why this method is virtual?

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:69:
NetworkStateHandler::Get()->InitShillPropertyHandler();
On 2013/05/02 22:11:13, pneubeck wrote:
> This last line shouldn't be necessary, if you instead swap the order of the
> first two statements.

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:76:
ethernet_network_ =
On 2013/05/02 22:11:13, pneubeck wrote:
> References to NetworkState should never be stored, not even in unit tests. 
> Please use only the service paths as identifier and refetch the state where
> necessary.

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl_unittest.cc:196:
base::Bind(&base::DoNothing), base::Bind(&ErrorCallbackFunction)
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> Please, combine lines #196 and #197.

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/login/network_state_informer.h (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/login/network_state_informer.h:10: 
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> #include "base/compiler_specific.h"

Done.

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:649:
params.SetString("lastNetworkType", connection_type);
On 2013/05/06 09:14:39, ygorshenin1 wrote:
> As you're passing connection_type as string instead of integer, please, fix 
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> 
> and
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

Actually, NetworkDropdown doesn't implement SetLastNetworkType anymore:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

So I went ahead and got rid of lastNetworkType as without SetLastNetworkType
implementation, there's no use for it in the JS. Let me know if I may have
missed something.

[ygorshenin1, 2013-05-08 08:03:23]

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):

https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:649:
params.SetString("lastNetworkType", connection_type);
Yes, lastNetworkType is not used, but we have incorrect comment for the JS code:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

and problems during casting from StringValue to double:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

Could you please fix comments and data types?

On 2013/05/07 23:38:10, gauravsh wrote:
> On 2013/05/06 09:14:39, ygorshenin1 wrote:
> > As you're passing connection_type as string instead of integer, please, fix 
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> > 
> > and
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> 
> Actually, NetworkDropdown doesn't implement SetLastNetworkType anymore:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> 
> So I went ahead and got rid of lastNetworkType as without SetLastNetworkType
> implementation, there's no use for it in the JS. Let me know if I may have
> missed something.

[gauravsh, 2013-05-08 18:33:58]

I fixed the comment and code in network_dropdown.js. I already fixed (removed)
the SetLastNetworkType call in network_dropdown_handler.cc.

On 2013/05/08 08:03:23, ygorshenin1 wrote:
>
https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
> File chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc (right):
> 
>
https://codereview.chromium.org/14134007/diff/12001/chrome/browser/ui/webui/c...
> chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc:649:
> params.SetString("lastNetworkType", connection_type);
> Yes, lastNetworkType is not used, but we have incorrect comment for the JS
code:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> 
> and problems during casting from StringValue to double:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> 
> Could you please fix comments and data types?
> 
> On 2013/05/07 23:38:10, gauravsh wrote:
> > On 2013/05/06 09:14:39, ygorshenin1 wrote:
> > > As you're passing connection_type as string instead of integer, please,
fix 
> > > 
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> > > 
> > > and
> > > 
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> > 
> > Actually, NetworkDropdown doesn't implement SetLastNetworkType anymore:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> > 
> > So I went ahead and got rid of lastNetworkType as without SetLastNetworkType
> > implementation, there's no use for it in the JS. Let me know if I may have
> > missed something.

[pneubeck (no reviews), 2013-05-08 18:41:39]

only one last comment from my side.
lgtm.

https://codereview.chromium.org/14134007/diff/7003/chromeos/network/network_s...
File chromeos/network/network_state_handler.h (right):

https://codereview.chromium.org/14134007/diff/7003/chromeos/network/network_s...
chromeos/network/network_state_handler.h:225: friend class
NetworkPortalDetectorImplTest;
not needed anymore.

[gauravsh, 2013-05-08 20:07:03]

https://codereview.chromium.org/14134007/diff/7003/chromeos/network/network_s...
File chromeos/network/network_state_handler.h (right):

https://codereview.chromium.org/14134007/diff/7003/chromeos/network/network_s...
chromeos/network/network_state_handler.h:225: friend class
NetworkPortalDetectorImplTest;
On 2013/05/08 18:41:39, pneubeck wrote:
> not needed anymore.

Done.

[gauravsh, 2013-05-09 17:38:10]

ygorshnenin: PTAL

[ygorshenin1, 2013-05-13 05:30:55]

lgtm

[gauravsh, 2013-05-13 20:18:45]

stevenjb: OWNERS review for chromeos/dbus/shill_service_client_stub.cc
nkostylev: Please do OWNERS review for c/b/chromeos/net/network_portal_detector*

[Nikita (slow), 2013-05-13 21:35:08]

lgtm

[stevenjb, 2013-05-14 15:46:15]

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_impl.cc (right):

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:104:
DCHECK(NetworkStateHandler::Get());
nit: This is redundant. The line below will CHECK that NetworkStateHandler has
been Initialized.

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:117: if
(NetworkStateHandler::Get())
Use NetworkStateHandler::IsInitialized(). Get() will CHECK if it has been
Shutdown.

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:206:
default_service_path_ = default_network->path();
nit: Skip if() here (or add if() above line 210 for consistency)

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:220:
DCHECK(default_network);
This DCHECK is redundant

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_stub.h (right):

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_stub.h:48: const
NetworkState* default_network_;
I know this is just a stub, but I'd still rather copy NetworkState than store a
pointer to one (or used a scoped_ptr and pass ownership).

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/p...
File chrome/browser/chromeos/proxy_config_service_impl.cc (right):

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/p...
chrome/browser/chromeos/proxy_config_service_impl.cc:577: const std::string&
proxy_config_string, net::ProxyConfig* proxy_config) {
args on separate lines

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/login/network_dropdown_handler.cc (right):

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/login/network_dropdown_handler.cc:60: const
std::string& element_id, bool oobe) {
args on separate lines

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/login/network_state_informer.cc (right):

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/login/network_state_informer.cc:112: if
(NetworkStateHandler::Get() &&
IsInitialized()

[gauravsh, 2013-05-14 21:51:15]

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_impl.cc (right):

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:104:
DCHECK(NetworkStateHandler::Get());
On 2013/05/14 15:46:16, stevenjb (chromium) wrote:
> nit: This is redundant. The line below will CHECK that NetworkStateHandler has
> been Initialized.

Done.

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:117: if
(NetworkStateHandler::Get())
On 2013/05/14 15:46:16, stevenjb (chromium) wrote:
> Use NetworkStateHandler::IsInitialized(). Get() will CHECK if it has been
> Shutdown.

Done.

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:206:
default_service_path_ = default_network->path();
On 2013/05/14 15:46:16, stevenjb (chromium) wrote:
> nit: Skip if() here (or add if() above line 210 for consistency)

Done.

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:220:
DCHECK(default_network);
On 2013/05/14 15:46:16, stevenjb (chromium) wrote:
> This DCHECK is redundant

Done.

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
File chrome/browser/chromeos/net/network_portal_detector_stub.h (right):

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_stub.h:48: const
NetworkState* default_network_;
On 2013/05/14 15:46:16, stevenjb (chromium) wrote:
> I know this is just a stub, but I'd still rather copy NetworkState than store
a
> pointer to one (or used a scoped_ptr and pass ownership).

I noticed that UpdateScreenBrowserTest doesn't really care for anything other
than the service path, so I removed the NetworkState dependency there and
changed the testing methods here to take the service path instead.

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/p...
File chrome/browser/chromeos/proxy_config_service_impl.cc (right):

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/chromeos/p...
chrome/browser/chromeos/proxy_config_service_impl.cc:577: const std::string&
proxy_config_string, net::ProxyConfig* proxy_config) {
On 2013/05/14 15:46:16, stevenjb (chromium) wrote:
> args on separate lines

Done.

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/login/network_dropdown_handler.cc (right):

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/login/network_dropdown_handler.cc:60: const
std::string& element_id, bool oobe) {
On 2013/05/14 15:46:16, stevenjb (chromium) wrote:
> args on separate lines

Done.

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/ui/webui/c...
File chrome/browser/ui/webui/chromeos/login/network_state_informer.cc (right):

https://codereview.chromium.org/14134007/diff/45028/chrome/browser/ui/webui/c...
chrome/browser/ui/webui/chromeos/login/network_state_informer.cc:112: if
(NetworkStateHandler::Get() &&
On 2013/05/14 15:46:16, stevenjb (chromium) wrote:
> IsInitialized()

Done.

[stevenjb, 2013-05-14 22:44:47]

It looks like the base upload failed, the deltas do not look correct.

[gauravsh, 2013-05-14 22:52:51]

On 2013/05/14 22:44:47, stevenjb (chromium) wrote:
> It looks like the base upload failed, the deltas do not look correct.

try now.

[stevenjb, 2013-05-17 18:30:55]

lgtm

[commit-bot: I haz the power, 2013-05-17 18:31:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gauravsh@chromium.org/14134007/76001

[commit-bot: I haz the power, 2013-05-17 18:31:47]

Failed to apply patch for
chrome/browser/chromeos/login/screens/update_screen.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/chromeos/login/screens/update_screen.cc
  Hunk #1 FAILED at 12.
  Hunk #2 succeeded at 21 (offset 1 line).
  Hunk #3 succeeded at 253 with fuzz 2 (offset 13 lines).
  Hunk #4 FAILED at 248.
  Hunk #5 succeeded at 518 with fuzz 1 (offset 32 lines).
  2 out of 5 hunks FAILED -- saving rejects to file
chrome/browser/chromeos/login/screens/update_screen.cc.rej

Patch:       chrome/browser/chromeos/login/screens/update_screen.cc
Index: chrome/browser/chromeos/login/screens/update_screen.cc
diff --git a/chrome/browser/chromeos/login/screens/update_screen.cc
b/chrome/browser/chromeos/login/screens/update_screen.cc
index
50058370daef11e6d065c7c35cadd51e8adf3268..0de1c7db7d8f3c2017831a969b2a7fbc652f0c68
100644
--- a/chrome/browser/chromeos/login/screens/update_screen.cc
+++ b/chrome/browser/chromeos/login/screens/update_screen.cc
@@ -12,7 +12,6 @@
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/threading/thread_restrictions.h"
-#include "chrome/browser/chromeos/cros/network_library.h"
 #include "chrome/browser/chromeos/login/screens/error_screen.h"
 #include "chrome/browser/chromeos/login/screens/screen_observer.h"
 #include "chrome/browser/chromeos/login/screens/update_screen_actor.h"
@@ -20,6 +19,7 @@
 #include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/network/network_state.h"
 #include "content/public/browser/browser_thread.h"
 
 using content::BrowserThread;
@@ -239,7 +239,7 @@ void UpdateScreen::UpdateStatusChanged(
 }
 
 void UpdateScreen::OnPortalDetectionCompleted(
-    const Network* network,
+    const NetworkState* network,
     const NetworkPortalDetector::CaptivePortalState& state) {
   // Wait for the sane portal detection results.
   if (!network ||
@@ -247,7 +247,7 @@ void UpdateScreen::OnPortalDetectionCompleted(
     return;
   }
   LOG(WARNING) << "UpdateScreen::OnPortalDetectionCompleted(): "
-               << "network=" << network->service_path() << ", "
+               << "network=" << network->path() << ", "
                << "state.status=" << state.status << ", "
                << "state.response_code=" << state.response_code;
   NetworkPortalDetector::CaptivePortalStatus status = state.status;
@@ -485,7 +485,7 @@ void UpdateScreen::HideErrorMessage() {
 }
 
 void UpdateScreen::UpdateErrorMessage(
-    const Network* network,
+    const NetworkState* network,
     const NetworkPortalDetector::CaptivePortalStatus status) {
   switch (status) {
     case NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_UNKNOWN:

[commit-bot: I haz the power, 2013-05-17 20:31:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gauravsh@chromium.org/14134007/87001

[commit-bot: I haz the power, 2013-05-18 00:19:32]

Retried try job too often on win_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-05-18 00:22:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gauravsh@chromium.org/14134007/87001

[commit-bot: I haz the power, 2013-05-18 08:48:24]

Change committed as 200968