Added stub portalled wifi network.

Added stub portalled wifi network.

Added stub portalled wifi network for testing captive portal UI on the
ChromeOS build. To use it one's need run chrome with the folliwing
switches:
--disable-stub-ethernet
--use-new-network-connection-handler
--default-stub-network-state-idle
--enable-stub-portalled-wifi

BUG=246391
TEST=manual tests on Lumpy

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

[oshima, 2013-08-06 21:00:01]

https://codereview.chromium.org/22364005/diff/1/chromeos/dbus/shill_service_c...
File chromeos/dbus/shill_service_client_stub.cc (right):

https://codereview.chromium.org/22364005/diff/1/chromeos/dbus/shill_service_c...
chromeos/dbus/shill_service_client_stub.cc:45: const char
ShillServiceClientStub::kStubPortalledWifiName[] = "Portalled Wifi";
If you follow my suggestion, you can put them in anonyous namespace.

https://codereview.chromium.org/22364005/diff/1/chromeos/dbus/shill_service_c...
chromeos/dbus/shill_service_client_stub.cc:190: state = flimflam::kStatePortal;
base::StringValue oneline_value(
   service_path.value() == kStubPortalledWifiPath ?
   flimflam::kStatePortal : flimflam::kStateOneline);

https://codereview.chromium.org/22364005/diff/1/chromeos/dbus/shill_service_c...
File chromeos/dbus/shill_service_client_stub.h (right):

https://codereview.chromium.org/22364005/diff/1/chromeos/dbus/shill_service_c...
chromeos/dbus/shill_service_client_stub.h:24: static const char
kStubPortalledWifiPath[];
Do you need to export both?

I'd also recommend to define predicates such as

bool IsStubPortalledWifiEnabled(const std::string& path)

That's cleaner and easier to read.

[ygorshenin1, 2013-08-07 09:00:30]

PTAL

https://codereview.chromium.org/22364005/diff/1/chromeos/dbus/shill_service_c...
File chromeos/dbus/shill_service_client_stub.cc (right):

https://codereview.chromium.org/22364005/diff/1/chromeos/dbus/shill_service_c...
chromeos/dbus/shill_service_client_stub.cc:45: const char
ShillServiceClientStub::kStubPortalledWifiName[] = "Portalled Wifi";
On 2013/08/06 21:00:02, oshima wrote:
> If you follow my suggestion, you can put them in anonyous namespace.

Done.

https://codereview.chromium.org/22364005/diff/1/chromeos/dbus/shill_service_c...
chromeos/dbus/shill_service_client_stub.cc:190: state = flimflam::kStatePortal;
On 2013/08/06 21:00:02, oshima wrote:
> base::StringValue oneline_value(
>    service_path.value() == kStubPortalledWifiPath ?
>    flimflam::kStatePortal : flimflam::kStateOneline);

Done.

https://codereview.chromium.org/22364005/diff/1/chromeos/dbus/shill_service_c...
File chromeos/dbus/shill_service_client_stub.h (right):

https://codereview.chromium.org/22364005/diff/1/chromeos/dbus/shill_service_c...
chromeos/dbus/shill_service_client_stub.h:24: static const char
kStubPortalledWifiPath[];
On 2013/08/06 21:00:02, oshima wrote:
> Do you need to export both?
> 
> I'd also recommend to define predicates such as
> 
> bool IsStubPortalledWifiEnabled(const std::string& path)
> 
> That's cleaner and easier to read.

Done.

[oshima, 2013-08-07 16:01:12]

lgtm if all comments addressed.

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

https://codereview.chromium.org/22364005/diff/10001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:13: #include
"chrome/common/chrome_switches.h"
this looks unnecessary too. can you remove it?

https://codereview.chromium.org/22364005/diff/10001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:14: #include
"chromeos/chromeos_switches.h"
nit: this is no longer necessary?

https://codereview.chromium.org/22364005/diff/10001/chromeos/dbus/shill_servi...
File chromeos/dbus/shill_service_client_stub.cc (right):

https://codereview.chromium.org/22364005/diff/10001/chromeos/dbus/shill_servi...
chromeos/dbus/shill_service_client_stub.cc:64: return (path ==
kStubPortalledWifiPath);
nit: nuke ()

https://codereview.chromium.org/22364005/diff/10001/chromeos/dbus/shill_servi...
chromeos/dbus/shill_service_client_stub.cc:321:
HasSwitch(chromeos::switches::kDefaultStubNetworkStateIdle)) {
nit: indent like this

CommandLine::ForCurrentProcess()->HasSwitch(
    chromeos::switches::kDefaultStubNetworkStateIdle)

this is more common and easier to read.

https://codereview.chromium.org/22364005/diff/10001/chromeos/dbus/shill_servi...
chromeos/dbus/shill_service_client_stub.cc:459:
HasSwitch(chromeos::switches::kEnableStubPortalledWifi)) {
ditto

[ygorshenin1, 2013-08-08 08:05:57]

Many thanks!

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

https://codereview.chromium.org/22364005/diff/10001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:13: #include
"chrome/common/chrome_switches.h"
On 2013/08/07 16:01:12, oshima wrote:
> this looks unnecessary too. can you remove it?

Done.

https://codereview.chromium.org/22364005/diff/10001/chrome/browser/chromeos/n...
chrome/browser/chromeos/net/network_portal_detector_impl.cc:14: #include
"chromeos/chromeos_switches.h"
On 2013/08/07 16:01:12, oshima wrote:
> nit: this is no longer necessary?

Done.

https://codereview.chromium.org/22364005/diff/10001/chromeos/dbus/shill_servi...
File chromeos/dbus/shill_service_client_stub.cc (right):

https://codereview.chromium.org/22364005/diff/10001/chromeos/dbus/shill_servi...
chromeos/dbus/shill_service_client_stub.cc:64: return (path ==
kStubPortalledWifiPath);
On 2013/08/07 16:01:12, oshima wrote:
> nit: nuke ()

Done.

https://codereview.chromium.org/22364005/diff/10001/chromeos/dbus/shill_servi...
chromeos/dbus/shill_service_client_stub.cc:321:
HasSwitch(chromeos::switches::kDefaultStubNetworkStateIdle)) {
On 2013/08/07 16:01:12, oshima wrote:
> nit: indent like this
> 
> CommandLine::ForCurrentProcess()->HasSwitch(
>     chromeos::switches::kDefaultStubNetworkStateIdle)
> 
> this is more common and easier to read.

Done.

https://codereview.chromium.org/22364005/diff/10001/chromeos/dbus/shill_servi...
chromeos/dbus/shill_service_client_stub.cc:459:
HasSwitch(chromeos::switches::kEnableStubPortalledWifi)) {
On 2013/08/07 16:01:12, oshima wrote:
> ditto

Done.

[commit-bot: I haz the power, 2013-08-08 10:36:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/22364005/18001

[commit-bot: I haz the power, 2013-08-08 19:40:35]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-08-09 14:42:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/22364005/18001

[commit-bot: I haz the power, 2013-08-09 15:07:53]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[ygorshenin1, 2013-08-12 09:12:48]

+ Nikita

Oshima, could you please help me?
I'm trying to land this CL, but still receive following compilation error from
chromeos trybots:
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds...

network_portal_detector_impl.cc is included into browser_chromeos static library
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/chrome_brow...),
which depends on chromeos/chromeos.gyp:chromeos target, so, there should not be
any problems with linkage, and when I compile this CL locally, everything is
fine.

Do you know what could cause the problem with compilation on trybots?

[oshima, 2013-08-12 14:25:15]

You need to export ShillServiceClientStub because they build with
component=shared_library.


On Mon, Aug 12, 2013 at 2:12 AM, <ygorshenin@chromium.org> wrote:

> + Nikita
>
> Oshima, could you please help me?
> I'm trying to land this CL, but still receive following compilation error
> from
> chromeos trybots:
> http://build.chromium.org/p/**tryserver.chromium/builders/**
>
linux_chromeos/builds/144732/**steps/compile/logs/stdio<http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds/144732/steps/compile/logs/stdio>
>
> network_portal_detector_impl.**cc is included into browser_chromeos
> static library
> (https://code.google.com/p/**chromium/codesearch#chromium/**
> src/chrome/chrome_browser_**chromeos.gypi&q=network_**
>
portal_detector_impl.cc&sq=**package:chromium&type=cs&l=9<https://code.google.com/p/chromium/codesearch#chromium/src/chrome/chrome_browser_chromeos.gypi&q=network_portal_detector_impl.cc&sq=package:chromium&type=cs&l=9>
> ),
> which depends on chromeos/chromeos.gyp:chromeos target, so, there should
> not be
> any problems with linkage, and when I compile this CL locally, everything
> is
> fine.
>
> Do you know what could cause the problem with compilation on trybots?
>
>
https://codereview.chromium.**org/22364005/<https://codereview.chromium.org/2...
>

[commit-bot: I haz the power, 2013-08-12 14:55:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ygorshenin@chromium.org/22364005/49001

[commit-bot: I haz the power, 2013-08-12 22:30:27]

Change committed as 217099

[ygorshenin1, 2013-08-13 07:49:17]

On 2013/08/12 14:25:15, oshima wrote:
> You need to export ShillServiceClientStub because they build with
> component=shared_library.

It helped! Many thanks!

>
> 
> On Mon, Aug 12, 2013 at 2:12 AM, <mailto:ygorshenin@chromium.org> wrote:
> 
> > + Nikita
> >
> > Oshima, could you please help me?
> > I'm trying to land this CL, but still receive following compilation error
> > from
> > chromeos trybots:
> > http://build.chromium.org/p/**tryserver.chromium/builders/**
> >
>
linux_chromeos/builds/144732/**steps/compile/logs/stdio<http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds/144732/steps/compile/logs/stdio>
> >
> > network_portal_detector_impl.**cc is included into browser_chromeos
> > static library
> > (https://code.google.com/p/**chromium/codesearch#chromium/**
> > src/chrome/chrome_browser_**chromeos.gypi&q=network_**
> >
>
portal_detector_impl.cc&sq=**package:chromium&type=cs&l=9<https://code.google.com/p/chromium/codesearch#chromium/src/chrome/chrome_browser_chromeos.gypi&q=network_portal_detector_impl.cc&sq=package:chromium&type=cs&l=9>
> > ),
> > which depends on chromeos/chromeos.gyp:chromeos target, so, there should
> > not be
> > any problems with linkage, and when I compile this CL locally, everything
> > is
> > fine.
> >
> > Do you know what could cause the problem with compilation on trybots?
> >
> >
>
https://codereview.chromium.**org/22364005/%3Chttps://codereview.chromium.org...>
> >