Add CheckPortalList manager property to NetworkStateHandler

Add CheckPortalList manager property to NetworkStateHandler

Also, gets rid of the code for handling this in NetworkLibrary and moves
WizardController to use NetworkStateHandler instead.

BUG=189035
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203296

[stevenjb, 2013-05-30 01:43:13]

lgtm w/ one small change

https://codereview.chromium.org/15899015/diff/1/chrome/browser/chromeos/login...
File chrome/browser/chromeos/login/wizard_controller_browsertest.cc (right):

https://codereview.chromium.org/15899015/diff/1/chrome/browser/chromeos/login...
chrome/browser/chromeos/login/wizard_controller_browsertest.cc:118:
NetworkHandler::Get()->network_state_handler()->InitShillPropertyHandler();
This should not be necessary

[gauravsh, 2013-05-30 18:27:37]

https://codereview.chromium.org/15899015/diff/1/chrome/browser/chromeos/login...
File chrome/browser/chromeos/login/wizard_controller_browsertest.cc (right):

https://codereview.chromium.org/15899015/diff/1/chrome/browser/chromeos/login...
chrome/browser/chromeos/login/wizard_controller_browsertest.cc:118:
NetworkHandler::Get()->network_state_handler()->InitShillPropertyHandler();
On 2013/05/30 01:43:13, stevenjb (chromium) wrote:
> This should not be necessary

Weird. If I remove this, the test starts failing. And I confirmed that
InitShillPropertyHandler is already being called as part of
NetworkHandler::Init().

[stevenjb, 2013-05-30 18:43:53]

https://codereview.chromium.org/15899015/diff/1/chrome/browser/chromeos/login...
File chrome/browser/chromeos/login/wizard_controller_browsertest.cc (right):

https://codereview.chromium.org/15899015/diff/1/chrome/browser/chromeos/login...
chrome/browser/chromeos/login/wizard_controller_browsertest.cc:118:
NetworkHandler::Get()->network_state_handler()->InitShillPropertyHandler();
On 2013/05/30 18:27:37, gauravsh wrote:
> On 2013/05/30 01:43:13, stevenjb (chromium) wrote:
> > This should not be necessary
> 
> Weird. If I remove this, the test starts failing. And I confirmed that
> InitShillPropertyHandler is already being called as part of
> NetworkHandler::Init().
How does it fail exactly?

[gauravsh, 2013-05-30 19:46:14]

On 2013/05/30 18:43:53, stevenjb (chromium) wrote:
>
https://codereview.chromium.org/15899015/diff/1/chrome/browser/chromeos/login...
> File chrome/browser/chromeos/login/wizard_controller_browsertest.cc (right):
> 
>
https://codereview.chromium.org/15899015/diff/1/chrome/browser/chromeos/login...
> chrome/browser/chromeos/login/wizard_controller_browsertest.cc:118:
> NetworkHandler::Get()->network_state_handler()->InitShillPropertyHandler();
> On 2013/05/30 18:27:37, gauravsh wrote:
> > On 2013/05/30 01:43:13, stevenjb (chromium) wrote:
> > > This should not be necessary
> > 
> > Weird. If I remove this, the test starts failing. And I confirmed that
> > InitShillPropertyHandler is already being called as part of
> > NetworkHandler::Init().
> How does it fail exactly?[ RUN      ] WizardControllerFlowTest.ControlFlowMain
See the log dump below. Looks like the SetCheckPortalList call in
WizardController is not resulting in NetworkStateHandler getting a
CheckPortalList update.


[31059:31059:0530/113953:3716703985994:ERROR:zygote_host_impl_linux.cc(144)]
Running without the SUID sandbox! See
https://code.google.com/p/chromium/wiki/LinuxSUIDSandboxDevelopment for more
information on dev
eloping with the sandbox on.
[31059:31059:0530/113953:3716703987331:WARNING:chrome_browser_main_chromeos.cc(441)]
Using new connection change notifier.
[31059:31059:0530/113953:3716704010046:ERROR:network_handler.cc(56)]
INITIALIZING NetworkHandler
[31059:31059:0530/113953:3716704025346:ERROR:nss_util.cc(492)] Error
initializing NSS with a persistent database (sql:/etc/fake_root_ca/nssdb): NSS
error code: -8174
[31059:31059:0530/113953:3716704032646:ERROR:network_state_handler.cc(85)]
INITIALIZING shill property handler
[31059:31059:0530/113953:3716704086519:INFO:cras_audio_handler.cc(460)]
Switching active device to: is_input = true id = 10004 display_name = Internal
Mic type = MIC active = false plugged_time= 0 
[31059:31059:0530/113953:3716704086604:WARNING:cras_audio_handler.cc(331)] input
mute state inconsistent, internal mute=0, dbus signal mute=1
[31059:31059:0530/113953:3716704154199:ERROR:component_loader.cc(106)] Failed to
parse extension manifest.
Xlib:  extension "RANDR" missing on display ":0".
[31059:31059:0530/113953:3716704377778:INFO:login_display_host_impl.cc(255)]
Login WebUI >> zero_delay: 1 wait_for_wp_load_: 0 wait_for_pods_: 0
init_webui_hidden_: 0
[31059:31059:0530/113953:3716704377988:INFO:login_display_host_impl.cc(377)]
Login WebUI >> wizard
[31059:31059:0530/113953:3716704406632:INFO:login_display_host_impl.cc(746)]
Login WebUI >> show login wnd on create
[31086:31086:0530/113953:ERROR:nss_util.cc(492)] Error initializing NSS with a
persistent database (sql:/etc/fake_root_ca/nssdb): NSS error code: -8174
[31115:31115:0530/113953:WARNING:sandbox_linux.cc(36)] Activated seccomp-bpf
sandbox for process type: renderer.
[31059:31059:0530/113953:3716704457086:ERROR:wizard_controller_browsertest.cc(126)]
0x19cbabf5b7a0
[31059:31059:0530/113953:3716704458027:INFO:wizard_controller.cc(711)] Wizard
screen exit code: 0
[31059:31059:0530/113953:3716704458273:INFO:wizard_controller.cc(711)] Wizard
screen exit code: 7
[31059:31059:0530/113953:3716704522832:INFO:wizard_controller.cc(711)] Wizard
screen exit code: 2
[31059:31059:0530/113953:3716704525753:INFO:login_display_host_impl.cc(407)]
Login WebUI >> sign in
../../chrome/browser/chromeos/login/wizard_controller_browsertest.cc:185:
Failure
Value of: NetworkHandler::Get()->network_state_handler()->check_portal_list()
  Actual: ""
Expected: "ethernet,wifi,cellular"
[31086:31086:0530/113953:WARNING:sandbox_linux.cc(36)] Activated seccomp-bpf
sandbox for process type: gpu-process.
[31059:31059:0530/113953:3716704704882:ERROR:network_handler.cc(64)] DESTROYING
NetworkHandler

[gauravsh, 2013-05-30 20:00:24]

On 2013/05/30 19:46:14, gauravsh wrote:
> On 2013/05/30 18:43:53, stevenjb (chromium) wrote:
> >
>
https://codereview.chromium.org/15899015/diff/1/chrome/browser/chromeos/login...
> > File chrome/browser/chromeos/login/wizard_controller_browsertest.cc (right):
> > 
> >
>
https://codereview.chromium.org/15899015/diff/1/chrome/browser/chromeos/login...
> > chrome/browser/chromeos/login/wizard_controller_browsertest.cc:118:
> > NetworkHandler::Get()->network_state_handler()->InitShillPropertyHandler();
> > On 2013/05/30 18:27:37, gauravsh wrote:
> > > On 2013/05/30 01:43:13, stevenjb (chromium) wrote:
> > > > This should not be necessary
> > > 
> > > Weird. If I remove this, the test starts failing. And I confirmed that
> > > InitShillPropertyHandler is already being called as part of
> > > NetworkHandler::Init().
> > How does it fail exactly?[ RUN      ]
WizardControllerFlowTest.ControlFlowMain
> See the log dump below. Looks like the SetCheckPortalList call in
> WizardController is not resulting in NetworkStateHandler getting a
> CheckPortalList update.
> 
> 
> [31059:31059:0530/113953:3716703985994:ERROR:zygote_host_impl_linux.cc(144)]
> Running without the SUID sandbox! See
> https://code.google.com/p/chromium/wiki/LinuxSUIDSandboxDevelopment for more
> information on dev
> eloping with the sandbox on.
>
[31059:31059:0530/113953:3716703987331:WARNING:chrome_browser_main_chromeos.cc(441)]
> Using new connection change notifier.
> [31059:31059:0530/113953:3716704010046:ERROR:network_handler.cc(56)]
> INITIALIZING NetworkHandler
> [31059:31059:0530/113953:3716704025346:ERROR:nss_util.cc(492)] Error
> initializing NSS with a persistent database (sql:/etc/fake_root_ca/nssdb): NSS
> error code: -8174
> [31059:31059:0530/113953:3716704032646:ERROR:network_state_handler.cc(85)]
> INITIALIZING shill property handler
> [31059:31059:0530/113953:3716704086519:INFO:cras_audio_handler.cc(460)]
> Switching active device to: is_input = true id = 10004 display_name = Internal
> Mic type = MIC active = false plugged_time= 0 
> [31059:31059:0530/113953:3716704086604:WARNING:cras_audio_handler.cc(331)]
input
> mute state inconsistent, internal mute=0, dbus signal mute=1
> [31059:31059:0530/113953:3716704154199:ERROR:component_loader.cc(106)] Failed
to
> parse extension manifest.
> Xlib:  extension "RANDR" missing on display ":0".
> [31059:31059:0530/113953:3716704377778:INFO:login_display_host_impl.cc(255)]
> Login WebUI >> zero_delay: 1 wait_for_wp_load_: 0 wait_for_pods_: 0
> init_webui_hidden_: 0
> [31059:31059:0530/113953:3716704377988:INFO:login_display_host_impl.cc(377)]
> Login WebUI >> wizard
> [31059:31059:0530/113953:3716704406632:INFO:login_display_host_impl.cc(746)]
> Login WebUI >> show login wnd on create
> [31086:31086:0530/113953:ERROR:nss_util.cc(492)] Error initializing NSS with a
> persistent database (sql:/etc/fake_root_ca/nssdb): NSS error code: -8174
> [31115:31115:0530/113953:WARNING:sandbox_linux.cc(36)] Activated seccomp-bpf
> sandbox for process type: renderer.
>
[31059:31059:0530/113953:3716704457086:ERROR:wizard_controller_browsertest.cc(126)]
> 0x19cbabf5b7a0
> [31059:31059:0530/113953:3716704458027:INFO:wizard_controller.cc(711)] Wizard
> screen exit code: 0
> [31059:31059:0530/113953:3716704458273:INFO:wizard_controller.cc(711)] Wizard
> screen exit code: 7
> [31059:31059:0530/113953:3716704522832:INFO:wizard_controller.cc(711)] Wizard
> screen exit code: 2
> [31059:31059:0530/113953:3716704525753:INFO:login_display_host_impl.cc(407)]
> Login WebUI >> sign in
> ../../chrome/browser/chromeos/login/wizard_controller_browsertest.cc:185:
> Failure
> Value of: NetworkHandler::Get()->network_state_handler()->check_portal_list()
>   Actual: ""
> Expected: "ethernet,wifi,cellular"
> [31086:31086:0530/113953:WARNING:sandbox_linux.cc(36)] Activated seccomp-bpf
> sandbox for process type: gpu-process.
> [31059:31059:0530/113953:3716704704882:ERROR:network_handler.cc(64)]
DESTROYING
> NetworkHandler

Never mind, I think I know that might be causing this. Calling Init on
ShillPropertyHandler again results in it re-requesting manager properties -
hence it correctly get the CheckPortalList update. Just need to figure out if
that's a race or a byproduct of how this particular test harness is setup.

Never mind, I think I figured out the issue. Calling
ShillPropertyHandler::Init() again results in

[gauravsh, 2013-05-30 20:48:55]

PTAL: The issue was that since the is using the manager client stub, setting a
property wouldn't trigger a property update (normally what shill would do).

I changed the manager client stub behavior to trigger a property update for all
observers when a manager property is changed. This would simulate usual shill
behavior.

[stevenjb, 2013-05-30 21:02:05]

Aha. LGTM

[commit-bot: I haz the power, 2013-05-30 21:02:58]

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

[commit-bot: I haz the power, 2013-05-31 01:16:40]

Change committed as 203296