Add queryForNewLocalDevices to gcdPrivate

Add function queryForNewLocalDevices to send query for new local devices,
implement it for mDNS (WiFi devices forthcoming)

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

[Vitaly Buka (NO REVIEWS), 2014-06-24 21:17:48]

lgtm

BUG?

[Vitaly Buka (NO REVIEWS), 2014-06-24 21:17:51]

The CQ bit was unchecked by vitalybuka@chromium.org

[asargent_no_longer_on_chrome, 2014-06-24 22:31:48]

https://codereview.chromium.org/356613002/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/gcd_private.idl (right):

https://codereview.chromium.org/356613002/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/gcd_private.idl:39: // onLocalDeviceStateChanged
event per device.
Did you consider having this method take a callback function that will get
called with the results, rather than making consumers of this API register for
the onLocalDeviceStateChanged event? I can imagine one advantage of using a
callback here would be that consumers would then know when the query had
finished; with the event only approach they don't ever know when it's done. (Not
sure if this is a useful concern or not though)

Also, a question - I would have assumed that onLocalDeviceStateChanged would
just fire automatically as new devices are discovered, and
queryForNewLocalDevices isn't needed. Is the point that queryForNewLocalDevices
just causes the system to immediately check for new devices sooner than we would
have based on some timer or something?

[Noam Samuel, 2014-06-24 23:07:06]

https://codereview.chromium.org/356613002/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/gcd_private.idl (right):

https://codereview.chromium.org/356613002/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/gcd_private.idl:39: // onLocalDeviceStateChanged
event per device.
On 2014/06/24 22:31:48, Antony Sargent wrote:
> Did you consider having this method take a callback function that will get
> called with the results, rather than making consumers of this API register for
> the onLocalDeviceStateChanged event? I can imagine one advantage of using a
> callback here would be that consumers would then know when the query had
> finished; with the event only approach they don't ever know when it's done.
(Not
> sure if this is a useful concern or not though)
> 
> Also, a question - I would have assumed that onLocalDeviceStateChanged would
> just fire automatically as new devices are discovered, and
> queryForNewLocalDevices isn't needed. Is the point that
queryForNewLocalDevices
> just causes the system to immediately check for new devices sooner than we
would
> have based on some timer or something?
> 

So registering for onLocalDeviceStateChanged causes the system to listen for
devices without generating network traffic. This means whenever a device
announces itself (either through an mDNS announce packet or by creating a new
WiFi network and sending a beacon frame), the event will be called immediately.

queryForNewLocalDevices generates a query on the network. This means sending an
mDNS query packets and (on platforms that support WiFi device discovery)
triggering a scan for new WiFi networks. The devices themselves will then reply
by announcing themselves, which will trigger onLocalDeviceStateChanged.

We could also send the query whenever the API user registers for
onLocalDeviceStateChanged, but we want to support the ability to have a refresh
button for re-querying the network.

[asargent_no_longer_on_chrome, 2014-06-24 23:12:49]

Ah, I get it, so there's really no concept of the query being "done". It might
be helpful for you to mention something like what you just said here on the CL
in a comment above the method. 

lgtm

[Vitaly Buka (NO REVIEWS), 2014-07-01 19:34:51]

The CQ bit was checked by vitalybuka@chromium.org

[commit-bot: I haz the power, 2014-07-01 19:37:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/noamsml@chromium.org/356613002/1

[commit-bot: I haz the power, 2014-07-01 19:49:25]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/8...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/23775)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/28179)

[commit-bot: I haz the power, 2014-07-01 19:50:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-01 19:50:36]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/23780)

[Noam Samuel, 2014-07-08 21:54:37]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-07-08 21:57:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/noamsml@chromium.org/356613002/20001

[commit-bot: I haz the power, 2014-07-09 01:51:25]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-07-09 02:56:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-09 02:56:52]

Try jobs failed on following builders:
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)

[Vitaly Buka (NO REVIEWS), 2014-07-09 04:34:09]

https://codereview.chromium.org/356613002/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/api/gcd_private/gcd_private_api.cc (right):

https://codereview.chromium.org/356613002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/api/gcd_private/gcd_private_api.cc:264: "You must
first subscribe to onCloudDeviceStateChanged notifications";
please update this

[Noam Samuel, 2014-07-09 17:01:36]

The CQ bit was checked by noamsml@chromium.org

[commit-bot: I haz the power, 2014-07-09 17:02:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/noamsml@chromium.org/356613002/80001

[commit-bot: I haz the power, 2014-07-09 19:35:09]

Change committed as 282098

[horo, 2014-07-10 01:24:32]

A revert of this CL has been created in
https://codereview.chromium.org/384503003/ by horo@chromium.org.

The reason for reverting is: Test failure in Builder: Win7 Tests (dbg)(6)

[ RUN      ] GcdPrivateAPITest.SendQuery
[4036:2900:0709/150928:ERROR:singleton_hwnd.cc(43)] Cannot create windows on
non-UI thread!
[956:2412:0709/150928:WARNING:extension_apitest.cc(282)] Workaround for 177163,
prematurely returning
c:\b\build\slave\win_builder__dbg_\build\src\chrome\browser\extensions\api\gcd_private\gcd_private_apitest.cc(276):
error: Actual function call count doesn't match
EXPECT_CALL(*test_service_discovery_client_,
OnSendTo(std::string(reinterpret_cast\u003Cconst char*>(kQueryPacket),
sizeof(kQueryPacket))))...
         Expected: to be called twice
           Actual: never called - unsatisfied and active
[  FAILED  ] GcdPrivateAPITest.SendQuery, where TypeParam =  and GetParam() = 
(5746 ms)

http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28...
http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28...
http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28....

[horo, 2014-07-10 01:43:36]

A revert of this CL has been created in
https://codereview.chromium.org/384543003/ by horo@chromium.org.

The reason for reverting is: Test failure in Builder: Win7 Tests (dbg)(6)

[  FAILED  ] GcdPrivateAPITest.SendQuery, where TypeParam =  and GetParam() = 
(5746 ms)

http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28...
http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28...
http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28....

[horo, 2014-07-10 02:32:00]

Revert button didn't work.
So I reverted with "drover --revert 282098".

https://codereview.chromium.org/377383008/