chromeos: Split NetworkChangeNotifierChromeos initialization.

chromeos: Split NetworkChangeNotifierChromeos initialization.

Defer the observer setup from the constructor to
chromeos_browser_main_chromeos.cc.

The motivation is to solve a dependency issue between
NetworkChangeNotifierChromeos and DBusThreadManager, that'll replace
CrosLibrary.

BUG=chromium-os:22459
TEST=networking on Chrome OS works as before.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=108564

[satorux1, 2011-11-03 17:31:54]

Try results from linux_chromeos bot looks depressing:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
but not sure if it's related to this patch.

Now running a new try job:
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds...

Meanwhile, please take a look at the patch.

[satorux1, 2011-11-03 17:45:46]

Forgot to add reviewers...

[stevenjb, 2011-11-03 19:26:17]

LGTM with one comment

http://codereview.chromium.org/8423077/diff/1/chrome/browser/chromeos/net/cro...
File chrome/browser/chromeos/net/cros_network_change_notifier_factory.cc
(right):

http://codereview.chromium.org/8423077/diff/1/chrome/browser/chromeos/net/cro...
chrome/browser/chromeos/net/cros_network_change_notifier_factory.cc:12:
NetworkChangeNotifierChromeos* g_network_change_notifier;
= NULL

[satorux1, 2011-11-03 19:30:27]

many tests in browser_tests failed again, though the results are different:

http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds...
vs
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds...

Will look into it...

http://codereview.chromium.org/8423077/diff/1/chrome/browser/chromeos/net/cro...
File chrome/browser/chromeos/net/cros_network_change_notifier_factory.cc
(right):

http://codereview.chromium.org/8423077/diff/1/chrome/browser/chromeos/net/cro...
chrome/browser/chromeos/net/cros_network_change_notifier_factory.cc:12:
NetworkChangeNotifierChromeos* g_network_change_notifier;
On 2011/11/03 19:26:17, Steven Bennetts wrote:
> = NULL

Done.

[satorux1, 2011-11-03 20:13:53]

http://codereview.chromium.org/8423077/diff/5001/chrome/browser/chromeos/net/...
File chrome/browser/chromeos/net/network_change_notifier_chromeos.cc (left):

http://codereview.chromium.org/8423077/diff/5001/chrome/browser/chromeos/net/...
chrome/browser/chromeos/net/network_change_notifier_chromeos.cc:76:
UpdateNetworkState(net);
So I thought the removal of the line 76 was harmless as net->active_network() is
NULL by the time this is called, which is true in production.

However, for tests on Linux desktop, net->active_network() is non-NULL here, and
removing the line breaks many tests in browser_tests.

Now thinking about how to fix this...

[satorux1, 2011-11-03 20:40:47]

PTAL

[stevenjb, 2011-11-03 20:47:56]

http://codereview.chromium.org/8423077/diff/6002/chrome/browser/chromeos/chro...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

http://codereview.chromium.org/8423077/diff/6002/chrome/browser/chromeos/chro...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:168:
network_change_notifier->Init(network_library);
I like this much better, but now we should move adding the observers to
NetworkChangeNotifierChromeos::Init(), since we remove them in
~NetworkChangeNotifierChromeos().

[satorux1, 2011-11-03 21:05:52]

http://codereview.chromium.org/8423077/diff/6002/chrome/browser/chromeos/chro...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

http://codereview.chromium.org/8423077/diff/6002/chrome/browser/chromeos/chro...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:168:
network_change_notifier->Init(network_library);
On 2011/11/03 20:47:56, Steven Bennetts wrote:
> I like this much better, but now we should move adding the observers to
> NetworkChangeNotifierChromeos::Init(), since we remove them in
> ~NetworkChangeNotifierChromeos().

Good catch. I forgot about removal of observers. I moved the observer setup to
NetworkChangeNotifierChromeos::Init()

[stevenjb, 2011-11-03 21:11:38]

LGTM!