Add a function to get Wifi SSID to net_util.

Add a function to get Wifi SSID to net_util.

This CL adds |GetWifiSSID| to net_util to get the SSID of the associated
WiFi access point's, if there is one. The function is currently only
implemented for Linux (including ChromeOS and Android). On Linux, it retrieves
WiFi SSIDs for each interface and if all interfaces have the same SSID, returns it as
the current SSID. This is similar to what https://codereview.chromium.org/739983005/ does.

BUG=451272
Committed: https://crrev.com/bc462ea514cba4bec764f5ae6a6593d6c8e57868
Cr-Commit-Position: refs/heads/master@{#316623}

[meacer, 2015-02-04 01:34:32]

meacer@chromium.org changed reviewers:
+ pauljensen@chromium.org

[meacer, 2015-02-04 01:34:33]

pauljensen: Can you PTAL?

I had another CL that added a similar method to NetworkChangeNotifier, but that
CL touches many more files compared to this one:
https://codereview.chromium.org/899573002/. Please let me know if you prefer
899573002 instead. Both of these are based on
https://codereview.chromium.org/739983005/ which you recently reviewed.

Thanks.

https://codereview.chromium.org/878513008/diff/20001/net/base/net_util_linux.cc
File net/base/net_util_linux.cc (right):

https://codereview.chromium.org/878513008/diff/20001/net/base/net_util_linux....
net/base/net_util_linux.cc:197: LOG(ERROR) << "Interface " << i << ": " <<
interfaces[i].name;
Debug cruft, to be removed.

[pauljensen, 2015-02-04 15:50:44]

I'm wondering if this NetworkInterfaceList->SSID API is appropriate.
Does the linux implementation work on Android, ChromeOS and Ubuntu?
I'm wondering how we'll implement on Windows.  WlanQueryInterface takes a
interface GUID.  I don't think we can map NetworkInterface to interface GUID
easily.  I should mention that GetNetworkList() on Windows can block for >1s.
I think not passing in a NetworkInterfaceList makes more sense for other OSs. 
WDYT about removing the NetworkInterfaceList argument?

[meacer, 2015-02-04 22:51:26]

On 2015/02/04 15:50:44, pauljensen wrote:
> I'm wondering if this NetworkInterfaceList->SSID API is appropriate.
> Does the linux implementation work on Android, ChromeOS and Ubuntu?
> I'm wondering how we'll implement on Windows.  WlanQueryInterface takes a
> interface GUID.  I don't think we can map NetworkInterface to interface GUID
> easily.  I should mention that GetNetworkList() on Windows can block for >1s.
> I think not passing in a NetworkInterfaceList makes more sense for other OSs. 
> WDYT about removing the NetworkInterfaceList argument?

WiFiService already provides SSID info on Windows and Mac which is why I left it
unimplemented here. This is the CL from last week to expose that info:
https://codereview.chromium.org/880143003/

I'm happy to remove the NetworkInterfaceList argument, and I'll if it works on
other platforms.

[meacer, 2015-02-06 20:55:12]

Removed NetworkInterfaceList. The function works on Android, ChromeOS and
Goobuntu. Can you PTAL?

[pauljensen, 2015-02-09 12:28:05]

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util.h
File net/base/net_util.h (right):

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util.h#newc...
net/base/net_util.h:499: // Gets the SSID of the currently associated access
point.
Might want to mention it only works if there is one.

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util.h#newc...
net/base/net_util.h:500: // Currently only available on OS_LINUX.
If this works on ChromeOS and Android, shouldn't we allow that too?

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_linux.cc
File net/base/net_util_linux.cc (right):

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_linux....
net/base/net_util_linux.cc:111: memset(&wreq, 0, sizeof(wreq));
Can you combine these two lines like line 87?

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_linux....
net/base/net_util_linux.cc:118: close(ioctl_socket);
Can you eliminate this line using ScopedFD like line 82?  I find this to be a
little safer as someone cannot add a "return" statement without forgetting to
close().

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_unitte...
File net/base/net_util_unittest.cc (right):

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_unitte...
net/base/net_util_unittest.cc:73: return (ifname == kInterfaceWithNoSSID) ? "" :
kWiFiSSID;
Instead of "", can this return something different from kWiFiSSID?  "" is just
confusing because this is what these functions return if things don't match up.

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_unitte...
net/base/net_util_unittest.cc:1358: }
Can we add a test for calling net::GetWiFiSSID() to test that it at least
doesn't crash?

[meacer, 2015-02-10 04:42:50]

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util.h
File net/base/net_util.h (right):

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util.h#newc...
net/base/net_util.h:499: // Gets the SSID of the currently associated access
point.
On 2015/02/09 12:28:04, pauljensen wrote:
> Might want to mention it only works if there is one.

Done.

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util.h#newc...
net/base/net_util.h:500: // Currently only available on OS_LINUX.
On 2015/02/09 12:28:05, pauljensen wrote:
> If this works on ChromeOS and Android, shouldn't we allow that too?

It actually already works on ChromeOS and Android without any change. I
clarified the comment.

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_linux.cc
File net/base/net_util_linux.cc (right):

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_linux....
net/base/net_util_linux.cc:111: memset(&wreq, 0, sizeof(wreq));
On 2015/02/09 12:28:05, pauljensen wrote:
> Can you combine these two lines like line 87?

Done.

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_linux....
net/base/net_util_linux.cc:118: close(ioctl_socket);
On 2015/02/09 12:28:05, pauljensen wrote:
> Can you eliminate this line using ScopedFD like line 82?  I find this to be a
> little safer as someone cannot add a "return" statement without forgetting to
> close().

Good point, done.

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_unitte...
File net/base/net_util_unittest.cc (right):

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_unitte...
net/base/net_util_unittest.cc:73: return (ifname == kInterfaceWithNoSSID) ? "" :
kWiFiSSID;
On 2015/02/09 12:28:05, pauljensen wrote:
> Instead of "", can this return something different from kWiFiSSID?  "" is just
> confusing because this is what these functions return if things don't match
up.

Done.

https://codereview.chromium.org/878513008/diff/40001/net/base/net_util_unitte...
net/base/net_util_unittest.cc:1358: }
On 2015/02/09 12:28:05, pauljensen wrote:
> Can we add a test for calling net::GetWiFiSSID() to test that it at least
> doesn't crash?

Renamed the above test and added GetWifiSSID.

[pauljensen, 2015-02-10 14:00:29]

Please update the commit description with the new function name, the fact that
it works on android and CrOS, etc.

[pauljensen, 2015-02-10 14:06:38]

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util.h
File net/base/net_util.h (right):

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util.h#new...
net/base/net_util.h:499: // Gets the SSID of the currently associated access
point if there is one.
associated access->associated WiFi access

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util.h#new...
net/base/net_util.h:501: // Currently only available on Linux, ChromeOS and
Android.
available->implemented

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util_linux.cc
File net/base/net_util_linux.cc (right):

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util_linux...
net/base/net_util_linux.cc:199: if (first) {
can we get rid of "first" and instead just use "i == 0" here?

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util_unitt...
File net/base/net_util_unittest.cc (right):

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util_unitt...
net/base/net_util_unittest.cc:1322: EXPECT_NE((const char*)NULL,
GetWifiSSID().c_str());
How come this doesn't hit the NOTIMPEMENTED() on Windows and Mac?

[meacer, 2015-02-11 00:24:53]

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util.h
File net/base/net_util.h (right):

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util.h#new...
net/base/net_util.h:499: // Gets the SSID of the currently associated access
point if there is one.
On 2015/02/10 14:06:38, pauljensen wrote:
> associated access->associated WiFi access

Done.

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util_linux.cc
File net/base/net_util_linux.cc (right):

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util_linux...
net/base/net_util_linux.cc:199: if (first) {
On 2015/02/10 14:06:38, pauljensen wrote:
> can we get rid of "first" and instead just use "i == 0" here?

Done.

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util_unitt...
File net/base/net_util_unittest.cc (right):

https://codereview.chromium.org/878513008/diff/100001/net/base/net_util_unitt...
net/base/net_util_unittest.cc:1322: EXPECT_NE((const char*)NULL,
GetWifiSSID().c_str());
On 2015/02/10 14:06:38, pauljensen wrote:
> How come this doesn't hit the NOTIMPEMENTED() on Windows and Mac?

Apparently NOTIMPLEMENTED() logs "NOTIMPLEMENTED" and doesn't assert which is
what I'm seeing in tests as well. 

NOTIMPLEMENTED_POLICY is only set to 0 (do nothing) or 4 (LOG(ERROR)):
https://code.google.com/p/chromium/codesearch#chromium/src/base/logging.h&q=N...

[meacer, 2015-02-13 20:21:19]

Friendly ping :)

[pauljensen, 2015-02-17 12:33:34]

lgtm

[meacer, 2015-02-17 18:02:03]

Thanks!

[meacer, 2015-02-17 18:02:07]

The CQ bit was checked by meacer@chromium.org

[commit-bot: I haz the power, 2015-02-17 18:03:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/878513008/120001

[commit-bot: I haz the power, 2015-02-17 19:33:01]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-02-17 19:33:49]

Patchset 7 (id:??) landed as
https://crrev.com/bc462ea514cba4bec764f5ae6a6593d6c8e57868
Cr-Commit-Position: refs/heads/master@{#316623}