Skip to content

Commit

Permalink
[M118] Add emergency off-switch for GetNetworkConnectivityHint API
Browse files Browse the repository at this point in the history
A previous CL added the call to GetNetworkConnectivityHint on
modern Windows systems, in order to function correctly inside
the sandbox.

This call makes subtle changes to the behavior of the network
change notifier, in particular it results in fewer incorrect
detections of online state, and more correct detections of
offline state.

As a precaution, place this new API call behind an
enabled-by-default emergency off-switch, just in case this has
any unexpected effects once it reaches Stable channel.

This CL contains no function change, since the feature is
enabled by default.

BUG=1450754

(cherry picked from commit 4bf4210)

Change-Id: I529bea07ee0a0a941574c04e0c2ca9440b72e33b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4903296
Commit-Queue: Will Harris <wfh@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1202977}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4908413
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Will Harris <wfh@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/branch-heads/5993@{#1069}
Cr-Branched-From: 5113507-refs/heads/main@{#1192594}
  • Loading branch information
Will Harris authored and Chromium LUCI CQ committed Oct 2, 2023
1 parent 5f0f452 commit b131a3b
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "content/public/browser/network_service_instance.h"
#include "content/public/common/content_features.h"
#include "content/public/test/browser_test.h"
#include "net/base/features.h"
#include "sandbox/policy/features.h"
#include "services/network/public/mojom/network_change_manager.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -137,7 +138,10 @@ class SandboxedNetworkChangeNotifierBrowserTest
SandboxedNetworkChangeNotifierBrowserTest() {
if (GetParam()) {
scoped_feature_list_.InitWithFeatures(
{sandbox::policy::features::kNetworkServiceSandbox},
{sandbox::policy::features::kNetworkServiceSandbox,
// When running inside the sandbox, the GetNetworkConnectivityHint
// API must be used.
net::features::kEnableGetNetworkConnectivityHintAPI},
{features::kNetworkServiceInProcess});
} else {
scoped_feature_list_.InitWithFeatures(
Expand Down
4 changes: 4 additions & 0 deletions net/base/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ BASE_FEATURE(kBlockNewForbiddenHeaders,
BASE_FEATURE(kPlatformKeyProbeSHA256,
"PlatformKeyProbeSHA256",
base::FEATURE_ENABLED_BY_DEFAULT);

BASE_FEATURE(kEnableGetNetworkConnectivityHintAPI,
"EnableGetNetworkConnectivityHintAPI",
base::FEATURE_ENABLED_BY_DEFAULT);
#endif

// Prefetch to follow normal semantics instead of 5-minute rule
Expand Down
4 changes: 4 additions & 0 deletions net/base/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,10 @@ NET_EXPORT BASE_DECLARE_FEATURE(kBlockNewForbiddenHeaders);
// Whether to probe for SHA-256 on some legacy platform keys, before assuming
// the key requires SHA-1. See SSLPlatformKeyWin for details.
NET_EXPORT BASE_DECLARE_FEATURE(kPlatformKeyProbeSHA256);

// Whether or not to use the GetNetworkConnectivityHint API on modern Windows
// versions for the Network Change Notifier.
NET_EXPORT BASE_DECLARE_FEATURE(kEnableGetNetworkConnectivityHintAPI);
#endif

// Prefetch to follow normal semantics instead of 5-minute rule
Expand Down
6 changes: 5 additions & 1 deletion net/base/network_change_notifier_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include <utility>

#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/location.h"
#include "base/logging.h"
Expand All @@ -20,6 +21,7 @@
#include "base/threading/thread.h"
#include "base/time/time.h"
#include "base/win/windows_version.h"
#include "net/base/features.h"
#include "net/base/winsock_init.h"
#include "net/base/winsock_util.h"

Expand Down Expand Up @@ -241,7 +243,9 @@ NetworkChangeNotifierWin::RecomputeCurrentConnectionTypeModern() {
// static
NetworkChangeNotifier::ConnectionType
NetworkChangeNotifierWin::RecomputeCurrentConnectionType() {
if (base::win::GetVersion() >= base::win::Version::WIN10_20H1) {
if (base::win::GetVersion() >= base::win::Version::WIN10_20H1 &&
base::FeatureList::IsEnabled(
features::kEnableGetNetworkConnectivityHintAPI)) {
return RecomputeCurrentConnectionTypeModern();
}

Expand Down

0 comments on commit b131a3b

Please sign in to comment.