Added ChromeOS-specific NetworkChangeNotifier specialization that is wired directly to flimflam.

chrome/browser/chromeos/net/network_change_notifier_chromeos.cc

+// Copyright (c) 2010 The Chromium Authors. All rights reserved.

[Sam Leffler, 2011/05/17 02:34:28]

it's 2011

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/chromeos/net/network_change_notifier_chromeos.h"
+
+#include <errno.h>
+#include <sys/socket.h>
+
+#include "base/compiler_specific.h"

[oshima, 2011/05/17 17:33:05]

move this to header (for OVERRIDE)

[zel, 2011/05/18 00:43:33]

Done.

+#include "base/eintr_wrapper.h"
+#include "base/task.h"

[oshima, 2011/05/17 17:33:05]

is this used?

[zel, 2011/05/18 00:43:33]

Done.

+#include "base/threading/thread.h"
+#include "chrome/browser/chromeos/cros/cros_library.h"
+#include "net/base/net_errors.h"
+
+namespace net {
+
+namespace {
+
+const int kInvalidSocket = -1;

[Sam Leffler, 2011/05/17 02:34:28]

don't see this used

[zel, 2011/05/18 00:43:33]

Done.

+
+}  // namespace
+
+NetworkChangeNotifierCros::NetworkChangeNotifierCros()
+    : connected_(false),
+      has_active_network_(false),
+      connectivity_state_(chromeos::CONN_STATE_UNKNOWN) {
+
+  chromeos::NetworkLibrary* lib =
+      chromeos::CrosLibrary::Get()->GetNetworkLibrary();
+  lib->AddNetworkManagerObserver(this);
+  UpdateNetworkState(lib);
+}
+
+NetworkChangeNotifierCros::~NetworkChangeNotifierCros() {
+  chromeos::NetworkLibrary* lib =
+      chromeos::CrosLibrary::Get()->GetNetworkLibrary();
+  lib->RemoveNetworkManagerObserver(this);
+}
+
+void NetworkChangeNotifierCros::OnNetworkManagerChanged(
+    chromeos::NetworkLibrary* cros) {
+  UpdateNetworkState(cros);
+}
+
+bool NetworkChangeNotifierCros::IsCurrentlyOffline() const {
+  return connectivity_state_ != chromeos::CONN_STATE_UNRESTRICTED;
+}
+
+void NetworkChangeNotifierCros::UpdateNetworkState(
+    chromeos::NetworkLibrary* lib) {
+  const chromeos::Network* network = lib->active_network();
+  // Check if the active network, its IP address or its connectivity status had
+  // changed.
+  if ((network && !has_active_network_) || (!network && has_active_network_) ||
+      (network && (network->service_path() != service_path_ ||
+                   network->connectivity_state() != connectivity_state_)) ||
+                   lib->IPAddress() != ip_address_) {

[oshima, 2011/05/17 17:33:05]

since the condition is a bit complicated, we need a test case to verify this.

[oshima, 2011/05/18 01:28:45]

The code looks correct, but I'm still concerned about lack of test as this is
last minute change. It's your call but I'd like to have some test at least
before m12 ships.

+    if (network) {
+      has_active_network_ = true;
+      service_path_ = network->service_path();
+    } else {
+      has_active_network_ = false;
+      service_path_.clear();
+    }
+    ip_address_ = lib->IPAddress();

[Sam Leffler, 2011/05/17 02:34:28]

I'm not familiar with this stuff but how do these conditions imply the ip
address changed?  Or is this notification mis-named?

[zel, 2011/05/18 00:43:33]

Yes, on ChromeOS side this notification should be interpreted as 'significant
connectivity change that requires invalidation of cached network-related browser
resources'. Linux version deals only with IP address change notifications from
netlink, hence the name.

+    NotifyObserversOfIPAddressChange();
+  }
+  if ((network && network->connectivity_state() != connectivity_state_) &&

[Sam Leffler, 2011/05/17 02:34:28]

Again, not familiar with semantics here, but flimflam delivers a boolean that
defines online/offline status; not sure why notification happens based on this
condition (no comments).  Also don't see how offline status ever gets delivered
via the notifier (network must always be != NULL)

[oshima, 2011/05/17 17:33:05]

If network can be null, don't we have to notify offline state when the state
becomes connected to network being null?

[zel, 2011/05/18 00:43:33]

This part is rewritten now. Please take a look at new set of network callbacks
routines. 

+      lib->Connected() != connected_) {
+    connectivity_state_ = network->connectivity_state();
+    connected_ = lib->Connected();
+    NotifyObserversOfOnlineStateChange();

[oshima, 2011/05/17 17:33:05]

I believe NetworkChangeNotifier clients expect to be called in IO thread. Is
this IO thread?

[zel, 2011/05/18 00:43:33]

Done.

+  }
+}
+
+}  // namespace net