Skip to content

Commit

Permalink
Reland "Reland "Use NetworkChangeNotifier to initialize the connectio…
Browse files Browse the repository at this point in the history
…n type in NetworkChangeManagerClient.""

This is a reland of 5db69e5
This CL exacerbated a threading issue in crrev.com/c/1538608, which was
reverted in crrev.com/c/1555375. Now that that's reverted, I'm relanding
this with no changes since it should be safe now.

[email protected],[email protected]

Original change's description:
> Reland "Use NetworkChangeNotifier to initialize the connection type in NetworkChangeManagerClient."
>
> This is a reland of d0310b0
>
> Original change's description:
> > Use NetworkChangeNotifier to initialize the connection type in NetworkChangeManagerClient.
> >
> > Bug: 942782
> > Change-Id: Ia2971348153d54cd87a48e8045efc9d31948db8a
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1529354
> > Reviewed-by: Steven Bennetts <[email protected]>
> > Commit-Queue: Robbie McElrath <[email protected]>
> > Cr-Commit-Position: refs/heads/master@{#641801}
>
> Bug: 942782
> Change-Id: Ie2331fe506b7b8839d9ae998bb3fd338747e0b24
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1529664
> Reviewed-by: Steven Bennetts <[email protected]>
> Reviewed-by: Paul Jensen <[email protected]>
> Commit-Queue: Robbie McElrath <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#642597}

Bug: 942782
Change-Id: Id48ccdf52ee7606d7bbf9ab7ed4f173cb627a54a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1555074
Reviewed-by: Robbie McElrath <[email protected]>
Commit-Queue: Robbie McElrath <[email protected]>
Cr-Commit-Position: refs/heads/master@{#650012}
  • Loading branch information
robbiemc authored and Commit Bot committed Apr 11, 2019
1 parent 5260fbc commit db03577
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 3 deletions.
5 changes: 3 additions & 2 deletions chrome/browser/chromeos/network_change_manager_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@
#include "chromeos/network/network_state_handler.h"
#include "content/public/browser/network_service_instance.h"
#include "content/public/common/network_service_util.h"
#include "net/base/network_change_notifier.h"
#include "net/base/network_change_notifier_posix.h"
#include "services/network/public/mojom/network_service.mojom.h"

namespace chromeos {

NetworkChangeManagerClient::NetworkChangeManagerClient(
net::NetworkChangeNotifierPosix* network_change_notifier)
: connection_type_(net::NetworkChangeNotifier::CONNECTION_NONE),
connection_subtype_(net::NetworkChangeNotifier::SUBTYPE_NONE),
: connection_type_(net::NetworkChangeNotifier::GetConnectionType()),
connection_subtype_(net::NetworkChangeNotifier::GetConnectionSubtype()),
network_change_notifier_(network_change_notifier) {
PowerManagerClient::Get()->AddObserver(this);
NetworkHandler::Get()->network_state_handler()->AddObserver(this, FROM_HERE);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/chromeos/network_change_manager_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ class NetworkChangeManagerClient
friend class NetworkChangeManagerClientUpdateTest;
FRIEND_TEST_ALL_PREFIXES(NetworkChangeManagerClientTest,
ConnectionTypeFromShill);
FRIEND_TEST_ALL_PREFIXES(NetworkChangeManagerClientTest,
NetworkChangeNotifierConnectionTypeUpdated);

void ConnectToNetworkChangeManager();
void ReconnectToNetworkChangeManager();
Expand Down
38 changes: 38 additions & 0 deletions chrome/browser/chromeos/network_change_manager_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/strings/string_split.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/power/power_manager_client.h"
#include "chromeos/dbus/shill/shill_service_client.h"
#include "chromeos/network/network_handler.h"
#include "chromeos/network/network_state.h"
#include "content/public/test/test_browser_thread_bundle.h"
Expand Down Expand Up @@ -106,6 +107,43 @@ TEST(NetworkChangeManagerClientTest, ConnectionTypeFromShill) {
}
}

TEST(NetworkChangeManagerClientTest,
NetworkChangeNotifierConnectionTypeUpdated) {
// Create a NetworkChangeNotifier with a non-NONE connection type.
content::TestBrowserThreadBundle thread_bundle_;
std::unique_ptr<net::NetworkChangeNotifierPosix> network_change_notifier(
static_cast<net::NetworkChangeNotifierPosix*>(
net::NetworkChangeNotifier::Create()));
network_change_notifier->OnConnectionChanged(
net::NetworkChangeNotifier::CONNECTION_UNKNOWN);
EXPECT_EQ(net::NetworkChangeNotifier::CONNECTION_UNKNOWN,
net::NetworkChangeNotifier::GetConnectionType());

// Initialize DBus and clear services so NetworkHandler thinks we're offline.
DBusThreadManager::Initialize();
PowerManagerClient::InitializeFake();
NetworkHandler::Initialize();
DBusThreadManager::Get()
->GetShillServiceClient()
->GetTestInterface()
->ClearServices();

auto client = std::make_unique<NetworkChangeManagerClient>(
network_change_notifier.get());

// NetworkChangeManagerClient should have read the network state from DBus
// and notified NetworkChangeNotifier that we're offline.
EXPECT_EQ(net::NetworkChangeNotifier::CONNECTION_NONE,
client->connection_type_);
EXPECT_EQ(net::NetworkChangeNotifier::CONNECTION_NONE,
net::NetworkChangeNotifier::GetConnectionType());

client.reset();
NetworkHandler::Shutdown();
PowerManagerClient::Shutdown();
DBusThreadManager::Shutdown();
}

class NetworkChangeManagerClientUpdateTest : public testing::Test {
protected:
NetworkChangeManagerClientUpdateTest() : default_network_("") {}
Expand Down
2 changes: 1 addition & 1 deletion net/base/network_change_notifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ NetworkChangeNotifier* NetworkChangeNotifier::Create() {
CHECK(false);
return NULL;
#elif defined(OS_CHROMEOS)
return new NetworkChangeNotifierPosix(CONNECTION_UNKNOWN, SUBTYPE_UNKNOWN);
return new NetworkChangeNotifierPosix(CONNECTION_NONE, SUBTYPE_NONE);
#elif defined(OS_LINUX)
return new NetworkChangeNotifierLinux(std::unordered_set<std::string>());
#elif defined(OS_MACOSX)
Expand Down

0 comments on commit db03577

Please sign in to comment.