This adds the onNetworkChanged event to networkingPrivate

This adds the onNetworkChanged event to networkingPrivate
extension API.

(TBR'ing trivial change to profile_dependency_manager.cc)

TBR=davemoore@chromium.org
BUG=chromium:168713
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181141

[Greg Spencer (Chromium), 2013-02-05 19:39:53]

pneubeck, please review from the networking UI perspective.

kalman, looking for an OWNERS review for extension code (which is basically all
of it).

davemoore, looking for an OWNERS review of:
chrome/browser/profiles/profile_dependency_manager.cc

[not at google - send to devlin, 2013-02-05 21:43:20]

https://codereview.chromium.org/12188027/diff/2002/chrome/browser/chromeos/ex...
File chrome/browser/chromeos/extensions/networking_private_event_router.cc
(right):

https://codereview.chromium.org/12188027/diff/2002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:40: }
nit: no {}

https://codereview.chromium.org/12188027/diff/2002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:55: }
no {}

https://codereview.chromium.org/12188027/diff/2002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:108: if
(listening_)
it can end up here if both should_listen and listening_ are true, in which case
removing the listener seems wrong.

https://codereview.chromium.org/12188027/diff/2002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:134:
changes.push_back(linked_ptr<api::NetworkProperties>(network_properties));
make_linked_ptr?

https://codereview.chromium.org/12188027/diff/2002/chrome/test/data/extension...
File chrome/test/data/extensions/api_test/networking/test.js (right):

https://codereview.chromium.org/12188027/diff/2002/chrome/test/data/extension...
chrome/test/data/extensions/api_test/networking/test.js:113: });
The event might (probably will) come in before the callback from startConnect
finishes. Need something like

var done = chrome.test.callbackAdded();
chrome.networkingPrivate.onNetworkChanged.addListener({
  ...
  done();
});

chrome.networkingPrivate.startConnect(...)

and I guess hope that the event only gets called once. Hm that's something we
should probably add, let's see how https://codereview.chromium.org/12218019
goes.

[Greg Spencer (Chromium), 2013-02-05 22:36:34]

https://codereview.chromium.org/12188027/diff/2002/chrome/browser/chromeos/ex...
File chrome/browser/chromeos/extensions/networking_private_event_router.cc
(right):

https://codereview.chromium.org/12188027/diff/2002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:40: }
On 2013/02/05 21:43:20, kalman wrote:
> nit: no {}

Done.

https://codereview.chromium.org/12188027/diff/2002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:55: }
On 2013/02/05 21:43:20, kalman wrote:
> no {}

Done.

https://codereview.chromium.org/12188027/diff/2002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:108: if
(listening_)
On 2013/02/05 21:43:20, kalman wrote:
> it can end up here if both should_listen and listening_ are true, in which
case
> removing the listener seems wrong.

Good catch!  Fixed.

https://codereview.chromium.org/12188027/diff/2002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:134:
changes.push_back(linked_ptr<api::NetworkProperties>(network_properties));
On 2013/02/05 21:43:20, kalman wrote:
> make_linked_ptr?

Done.

https://codereview.chromium.org/12188027/diff/2002/chrome/test/data/extension...
File chrome/test/data/extensions/api_test/networking/test.js (right):

https://codereview.chromium.org/12188027/diff/2002/chrome/test/data/extension...
chrome/test/data/extensions/api_test/networking/test.js:113: });
On 2013/02/05 21:43:20, kalman wrote:
> The event might (probably will) come in before the callback from startConnect
> finishes. Need something like
> 
> var done = chrome.test.callbackAdded();
> chrome.networkingPrivate.onNetworkChanged.addListener({
>   ...
>   done();
> });
> 
> chrome.networkingPrivate.startConnect(...)
> 
> and I guess hope that the event only gets called once. Hm that's something we
> should probably add, let's see how https://codereview.chromium.org/12218019
> goes.

Thanks, I was wondering how to do that.  Done.

[not at google - send to devlin, 2013-02-05 22:38:07]

lgtm

[pneubeck (no reviews), 2013-02-05 23:08:30]

only nits.
LGTM.

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
File chrome/browser/chromeos/extensions/networking_private_event_router.cc
(right):

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:91: // If
there are any listeners for our event, then we want to register for
move/combine this comment to the header?

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:97:
CHECK(event_router);
nit: not necessary, the calls below would fail immediately.

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:107: if
(listening_)
else if (...

outside of clause
 listening_ = should_listen

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:115:
extensions::EventRouter* event_router =
might move that to its usage below

nit: again, check shouldn't be necessary

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
File chrome/browser/chromeos/extensions/networking_private_event_router.h
(right):

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.h:38:
private:
nit: empty line above

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/extensions/...
File chrome/browser/extensions/event_names.cc (right):

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/extensions/...
chrome/browser/extensions/event_names.cc:97: const char kOnNetworkChanged[] =
one line

[Greg Spencer (Chromium), 2013-02-05 23:17:35]

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
File chrome/browser/chromeos/extensions/networking_private_event_router.cc
(right):

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:91: // If
there are any listeners for our event, then we want to register for
On 2013/02/05 23:08:30, pneubeck wrote:
> move/combine this comment to the header?

Done.

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:97:
CHECK(event_router);
On 2013/02/05 23:08:30, pneubeck wrote:
> nit: not necessary, the calls below would fail immediately.

Done.

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:107: if
(listening_)
On 2013/02/05 23:08:30, pneubeck wrote:
> else if (...
> 
> outside of clause
>  listening_ = should_listen

Done.

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.cc:115:
extensions::EventRouter* event_router =
On 2013/02/05 23:08:30, pneubeck wrote:
> might move that to its usage below
> 
> nit: again, check shouldn't be necessary

Done.

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
File chrome/browser/chromeos/extensions/networking_private_event_router.h
(right):

https://codereview.chromium.org/12188027/diff/9002/chrome/browser/chromeos/ex...
chrome/browser/chromeos/extensions/networking_private_event_router.h:38:
private:
On 2013/02/05 23:08:30, pneubeck wrote:
> nit: empty line above

Done.

[commit-bot: I haz the power, 2013-02-05 23:26:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gspencer@chromium.org/12188027/3014

[commit-bot: I haz the power, 2013-02-06 19:16:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gspencer@chromium.org/12188027/5017

[commit-bot: I haz the power, 2013-02-06 19:41:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gspencer@chromium.org/12188027/5018

[commit-bot: I haz the power, 2013-02-06 20:14:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gspencer@chromium.org/12188027/9007

[commit-bot: I haz the power, 2013-02-07 02:03:07]

Change committed as 181141