Invoke createNetwork() callback with GUID, not service name

Invoke createNetwork() callback with GUID, not service name

The current implementation doesn't match the API spec.  Fix this.

BUG=593196
TEST=go to chrome://settings/network , ctrl-shift-i, run in console:
     chrome.networkingPrivate.createNetwork(false,
         {"Type":"WiFi","WiFi":{"SSID":"GoogleGuest","AutoConnect":true}},
         function(x) { console.log(x); })

Committed: https://crrev.com/f1da84b10359d62c67a73e46da90b0d0510a0768
Cr-Commit-Position: refs/heads/master@{#381347}

[Kevin Cernekee, 2016-03-10 01:04:41]

cernekee@chromium.org changed reviewers:
+ abhishekbh@google.com, stevenjb@chromium.org

[stevenjb, 2016-03-10 01:49:10]

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
File extensions/browser/api/networking_private/networking_private_chromeos.cc
(right):

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
extensions/browser/api/networking_private/networking_private_chromeos.cc:319:
user_id_hash, *properties, success_callback,
By passing |success_callback| here directly, we are imposing the Chrome OS
implementation detail to the extension API. Instead we should create a local
wrapper function that passes just |guid| from the CreateConfiguration() callback
to |success_callback| (which should continue to just take a guid). Does that
make sense?

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
File extensions/browser/api/networking_private/networking_private_delegate.h
(right):

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
extensions/browser/api/networking_private/networking_private_delegate.h:41:
base::Callback<void(const std::string&, const std::string&)>;
'ServiceCallback' isn't very meaningful in this context, plus I think we can get
away with just using 'guid' here. See my comment in
NetworkingPrivateChromeOS::CreateNetwork.

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
File
extensions/browser/api/networking_private/networking_private_service_client.h
(right):

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
extensions/browser/api/networking_private/networking_private_service_client.h:69:
const FailureCallback& failure_callback) override;
This will almost certainly break on Windows.

[Kevin Cernekee, 2016-03-11 01:53:58]

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
File extensions/browser/api/networking_private/networking_private_chromeos.cc
(right):

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
extensions/browser/api/networking_private/networking_private_chromeos.cc:319:
user_id_hash, *properties, success_callback,
On 2016/03/10 01:49:10, stevenjb wrote:
> By passing |success_callback| here directly, we are imposing the Chrome OS
> implementation detail to the extension API. Instead we should create a local
> wrapper function that passes just |guid| from the CreateConfiguration()
callback
> to |success_callback| (which should continue to just take a guid). Does that
> make sense?

Done.

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
File extensions/browser/api/networking_private/networking_private_delegate.h
(right):

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
extensions/browser/api/networking_private/networking_private_delegate.h:41:
base::Callback<void(const std::string&, const std::string&)>;
On 2016/03/10 01:49:10, stevenjb wrote:
> 'ServiceCallback' isn't very meaningful in this context, plus I think we can
get
> away with just using 'guid' here. See my comment in
> NetworkingPrivateChromeOS::CreateNetwork.

Done.

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
File
extensions/browser/api/networking_private/networking_private_service_client.h
(right):

https://codereview.chromium.org/1779633002/diff/20001/extensions/browser/api/...
extensions/browser/api/networking_private/networking_private_service_client.h:69:
const FailureCallback& failure_callback) override;
On 2016/03/10 01:49:10, stevenjb wrote:
> This will almost certainly break on Windows.

Done.

[stevenjb, 2016-03-12 01:52:54]

lgtm

[Kevin Cernekee, 2016-03-12 01:53:29]

The CQ bit was checked by cernekee@chromium.org

[commit-bot: I haz the power, 2016-03-12 01:53:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1779633002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1779633002/80001

[commit-bot: I haz the power, 2016-03-12 02:04:44]

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

[commit-bot: I haz the power, 2016-03-12 02:04:45]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Kevin Cernekee, 2016-03-12 02:10:04]

cernekee@chromium.org changed reviewers:
+ bartfab@chromium.org, jochen@chromium.org, zea@chromium.org

[Kevin Cernekee, 2016-03-12 02:10:04]

Hi guys, need a couple more OWNERS approvals for this minor API change.  Could
you please review?

Thanks!

[jochen (gone - plz use gerrit), 2016-03-14 14:24:18]

what files do you want me to review?

[Kevin Cernekee, 2016-03-14 19:10:28]

On 2016/03/14 14:24:18, jochen wrote:
> what files do you want me to review?

jochen - could you please take a look at the files under components/wifi_sync ?

Thanks!

[Nicolas Zea, 2016-03-14 22:43:00]

chrome/browser/sync lgtm

[jochen (gone - plz use gerrit), 2016-03-15 21:01:20]

lgtm

[Kevin Cernekee, 2016-03-15 21:03:23]

cernekee@chromium.org changed reviewers:
+ emaxx@chromium.org

[Kevin Cernekee, 2016-03-15 21:03:24]

+emaxx for VPN review

[emaxx, 2016-03-15 22:31:29]

On 2016/03/15 21:03:24, Kevin Cernekee wrote:
> +emaxx for VPN review

lgtm for VPN

[Kevin Cernekee, 2016-03-15 22:39:43]

The CQ bit was checked by cernekee@chromium.org

[commit-bot: I haz the power, 2016-03-15 22:40:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1779633002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1779633002/80001

[commit-bot: I haz the power, 2016-03-15 23:33:18]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-15 23:35:20]

Description was changed from

==========
Invoke createNetwork() callback with GUID, not service name

The current implementation doesn't match the API spec.  Fix this.

BUG=593196
TEST=go to chrome://settings/network , ctrl-shift-i, run in console:
     chrome.networkingPrivate.createNetwork(false,
         {"Type":"WiFi","WiFi":{"SSID":"GoogleGuest","AutoConnect":true}},
         function(x) { console.log(x); })
==========

to

==========
Invoke createNetwork() callback with GUID, not service name

The current implementation doesn't match the API spec.  Fix this.

BUG=593196
TEST=go to chrome://settings/network , ctrl-shift-i, run in console:
     chrome.networkingPrivate.createNetwork(false,
         {"Type":"WiFi","WiFi":{"SSID":"GoogleGuest","AutoConnect":true}},
         function(x) { console.log(x); })

Committed: https://crrev.com/f1da84b10359d62c67a73e46da90b0d0510a0768
Cr-Commit-Position: refs/heads/master@{#381347}
==========

[commit-bot: I haz the power, 2016-03-15 23:35:21]

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