[BackgroundSync] Remove the requested network state from registration

We used to support sync registrations having various types of network
conditions under which they would fire. That's not specced and it's
unused, so removing it.

Bug: 901034
Change-Id: I38fb1cf2779aebe8814f7bb1bf5564f467397149
Reviewed-on: https://chromium-review.googlesource.com/c/1313191
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605448}

content/browser/background_sync/background_sync.proto

@@ -19,7 +19,7 @@ message BackgroundSyncRegistrationProto {
   required string tag = 2;
   // required SyncPeriodicity periodicity = 3;
   // required int64 min_period = 4;
-  required SyncNetworkState network_state = 5;
+  // required SyncNetworkState network_state = 5;
   // required SyncPowerState power_state = 6;
   required int32 num_attempts = 7;
   required int64 delay_until = 8;

content/browser/background_sync/background_sync_manager.cc

@@ -400,7 +400,6 @@ void BackgroundSyncManager::InitDidGetDataFromBackend(
 
         BackgroundSyncRegistrationOptions* options = registration->options();
         options->tag = registration_proto.tag();
-        options->network_state = registration_proto.network_state();
 
         registration->set_num_attempts(registration_proto.num_attempts());
         registration->set_delay_until(
@@ -655,8 +654,6 @@ void BackgroundSyncManager::StoreRegistrations(
     BackgroundSyncRegistrationProto* registration_proto =
         registrations_proto.add_registration();
     registration_proto->set_tag(registration.options()->tag);
-    registration_proto->set_network_state(
-        registration.options()->network_state);
     registration_proto->set_num_attempts(registration.num_attempts());
     registration_proto->set_delay_until(
         registration.delay_until().ToInternalValue());
@@ -837,7 +834,7 @@ void BackgroundSyncManager::GetRegistrationsImpl(
 bool BackgroundSyncManager::AreOptionConditionsMet(
     const BackgroundSyncRegistrationOptions& options) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  return network_observer_->NetworkSufficient(options.network_state);
+  return network_observer_->NetworkSufficient();
 }
 
 bool BackgroundSyncManager::IsRegistrationReadyToFire(
@@ -1204,7 +1201,7 @@ blink::ServiceWorkerStatusCode BackgroundSyncManager::CanEmulateSyncEvent(
     scoped_refptr<ServiceWorkerVersion> active_version) {
   if (!active_version)
     return blink::ServiceWorkerStatusCode::kErrorFailed;
-  if (!network_observer_->NetworkSufficient(NETWORK_STATE_ONLINE))
+  if (!network_observer_->NetworkSufficient())
     return blink::ServiceWorkerStatusCode::kErrorNetwork;
   int64_t registration_id = active_version->registration_id();
   if (base::ContainsKey(emulated_offline_sw_, registration_id))

content/browser/background_sync/background_sync_manager_unittest.cc

@@ -88,10 +88,7 @@ class BackgroundSyncManagerTest : public testing::Test {
   BackgroundSyncManagerTest()
       : browser_thread_bundle_(TestBrowserThreadBundle::IO_MAINLOOP) {
     sync_options_1_.tag = "foo";
-    sync_options_1_.network_state = NETWORK_STATE_ONLINE;
-
     sync_options_2_.tag = "bar";
-    sync_options_2_.network_state = NETWORK_STATE_ONLINE;
   }
 
   void SetUp() override {
@@ -744,23 +741,12 @@ TEST_F(BackgroundSyncManagerTest, RegistrationEqualsTag) {
   EXPECT_FALSE(reg_1.Equals(reg_2));
 }
 
-TEST_F(BackgroundSyncManagerTest, RegistrationEqualsNetworkState) {
-  BackgroundSyncRegistration reg_1;
-  BackgroundSyncRegistration reg_2;
-  EXPECT_TRUE(reg_1.Equals(reg_2));
-  reg_1.options()->network_state = NETWORK_STATE_ANY;
-  reg_2.options()->network_state = NETWORK_STATE_ONLINE;
-  EXPECT_FALSE(reg_1.Equals(reg_2));
-}
-
 TEST_F(BackgroundSyncManagerTest, StoreAndRetrievePreservesValues) {
   InitDelayedSyncEventTest();
   BackgroundSyncRegistrationOptions options;
 
   // Set non-default values for each field.
   options.tag = "foo";
-  EXPECT_NE(NETWORK_STATE_AVOID_CELLULAR, options.network_state);
-  options.network_state = NETWORK_STATE_AVOID_CELLULAR;
 
   // Store the registration.
   EXPECT_TRUE(Register(options));

content/browser/background_sync/background_sync_network_observer.cc

@@ -67,26 +67,10 @@ void BackgroundSyncNetworkObserver::UpdateConnectionType() {
     OnConnectionChanged(connection_type);
 }
 
-bool BackgroundSyncNetworkObserver::NetworkSufficient(
-    SyncNetworkState network_state) {
+bool BackgroundSyncNetworkObserver::NetworkSufficient() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  switch (network_state) {
-    case NETWORK_STATE_ANY:
-      return true;
-    case NETWORK_STATE_AVOID_CELLULAR:
-      // Note that this returns true for CONNECTION_UNKNOWN to avoid never
-      // firing.
-      return connection_type_ !=
-                 network::mojom::ConnectionType::CONNECTION_NONE &&
-             !network::NetworkConnectionTracker::IsConnectionCellular(
-                 connection_type_);
-    case NETWORK_STATE_ONLINE:
-      return connection_type_ !=
-             network::mojom::ConnectionType::CONNECTION_NONE;
-  }
-
-  NOTREACHED();
+  return connection_type_ != network::mojom::ConnectionType::CONNECTION_NONE;
   return false;
 }
 

content/browser/background_sync/background_sync_network_observer.h

@@ -28,9 +28,8 @@ class CONTENT_EXPORT BackgroundSyncNetworkObserver
   // (For preventing flakes in tests)
   static void SetIgnoreNetworkChangesForTests(bool ignore);
 
-  // Returns true if the state of the network meets the needs of
-  // |network_state|.
-  bool NetworkSufficient(SyncNetworkState network_state);
+  // Returns true if the network is online.
+  bool NetworkSufficient();
 
   // NetworkConnectionObserver overrides
   void OnConnectionChanged(

content/browser/background_sync/background_sync_network_observer_unittest.cc

@@ -51,58 +51,18 @@ TEST_F(BackgroundSyncNetworkObserverTest, NetworkChangeInvokesCallback) {
   EXPECT_EQ(4, network_changed_count_);
 }
 
-TEST_F(BackgroundSyncNetworkObserverTest, NetworkSufficientAnyNetwork) {
-  SetNetwork(network::mojom::ConnectionType::CONNECTION_WIFI);
-  EXPECT_TRUE(network_observer_->NetworkSufficient(NETWORK_STATE_ANY));
-
-  SetNetwork(network::mojom::ConnectionType::CONNECTION_3G);
-  EXPECT_TRUE(network_observer_->NetworkSufficient(NETWORK_STATE_ANY));
-
-  SetNetwork(network::mojom::ConnectionType::CONNECTION_UNKNOWN);
-  EXPECT_TRUE(network_observer_->NetworkSufficient(NETWORK_STATE_ANY));
-
-  SetNetwork(network::mojom::ConnectionType::CONNECTION_NONE);
-  EXPECT_TRUE(network_observer_->NetworkSufficient(NETWORK_STATE_ANY));
-}
-
-TEST_F(BackgroundSyncNetworkObserverTest, NetworkSufficientAvoidCellular) {
-  SetNetwork(network::mojom::ConnectionType::CONNECTION_WIFI);
-  EXPECT_TRUE(
-      network_observer_->NetworkSufficient(NETWORK_STATE_AVOID_CELLULAR));
-
-  SetNetwork(network::mojom::ConnectionType::CONNECTION_UNKNOWN);
-  EXPECT_TRUE(
-      network_observer_->NetworkSufficient(NETWORK_STATE_AVOID_CELLULAR));
-
-  SetNetwork(network::mojom::ConnectionType::CONNECTION_2G);
-  EXPECT_FALSE(
-      network_observer_->NetworkSufficient(NETWORK_STATE_AVOID_CELLULAR));
-
-  SetNetwork(network::mojom::ConnectionType::CONNECTION_3G);
-  EXPECT_FALSE(
-      network_observer_->NetworkSufficient(NETWORK_STATE_AVOID_CELLULAR));
-
-  SetNetwork(network::mojom::ConnectionType::CONNECTION_4G);
-  EXPECT_FALSE(
-      network_observer_->NetworkSufficient(NETWORK_STATE_AVOID_CELLULAR));
-
-  SetNetwork(network::mojom::ConnectionType::CONNECTION_NONE);
-  EXPECT_FALSE(
-      network_observer_->NetworkSufficient(NETWORK_STATE_AVOID_CELLULAR));
-}
-
 TEST_F(BackgroundSyncNetworkObserverTest, ConditionsMetOnline) {
   SetNetwork(network::mojom::ConnectionType::CONNECTION_WIFI);
-  EXPECT_TRUE(network_observer_->NetworkSufficient(NETWORK_STATE_ONLINE));
+  EXPECT_TRUE(network_observer_->NetworkSufficient());
 
   SetNetwork(network::mojom::ConnectionType::CONNECTION_3G);
-  EXPECT_TRUE(network_observer_->NetworkSufficient(NETWORK_STATE_ONLINE));
+  EXPECT_TRUE(network_observer_->NetworkSufficient());
 
   SetNetwork(network::mojom::ConnectionType::CONNECTION_UNKNOWN);
-  EXPECT_TRUE(network_observer_->NetworkSufficient(NETWORK_STATE_ONLINE));
+  EXPECT_TRUE(network_observer_->NetworkSufficient());
 
   SetNetwork(network::mojom::ConnectionType::CONNECTION_NONE);
-  EXPECT_FALSE(network_observer_->NetworkSufficient(NETWORK_STATE_ONLINE));
+  EXPECT_FALSE(network_observer_->NetworkSufficient());
 }
 
 TEST_F(BackgroundSyncNetworkObserverTest, GetNetworkOnConstruction) {
@@ -120,7 +80,7 @@ TEST_F(BackgroundSyncNetworkObserverTest, GetNetworkOnConstruction) {
   // The network observer should have learned that the current network is not
   // the default (UNKNOWN) but is in fact NONE.
   EXPECT_EQ(1, network_changed_count_);
-  EXPECT_FALSE(observer->NetworkSufficient(NETWORK_STATE_ONLINE));
+  EXPECT_FALSE(observer->NetworkSufficient());
 }
 
 }  // namespace content

content/browser/background_sync/background_sync_registration_options.cc

@@ -8,7 +8,7 @@ namespace content {
 
 bool BackgroundSyncRegistrationOptions::Equals(
     const BackgroundSyncRegistrationOptions& other) const {
-  return tag == other.tag && network_state == other.network_state;
+  return tag == other.tag;
 }
 
 }  // namespace content

content/browser/background_sync/background_sync_registration_options.h

@@ -19,7 +19,6 @@ struct CONTENT_EXPORT BackgroundSyncRegistrationOptions {
   bool Equals(const BackgroundSyncRegistrationOptions& other) const;
 
   std::string tag;
-  SyncNetworkState network_state = NETWORK_STATE_ONLINE;
 };
 
 }  // namespace content

content/browser/background_sync/background_sync_service_impl.cc

@@ -23,7 +23,6 @@ BackgroundSyncRegistrationOptions ToBackgroundSyncRegistrationOptions(
   BackgroundSyncRegistrationOptions out;
 
   out.tag = in->tag;
-  out.network_state = static_cast<SyncNetworkState>(in->network_state);
   return out;
 }
 
@@ -31,8 +30,6 @@ blink::mojom::SyncRegistrationPtr ToMojoRegistration(
     const BackgroundSyncRegistration& in) {
   blink::mojom::SyncRegistrationPtr out(blink::mojom::SyncRegistration::New());
   out->tag = in.options()->tag;
-  out->network_state = static_cast<blink::mojom::BackgroundSyncNetworkState>(
-      in.options()->network_state);
   return out;
 }
 
@@ -59,15 +56,6 @@ COMPILE_ASSERT_MATCHING_ENUM(mojom::BackgroundSyncError::PERMISSION_DENIED,
 COMPILE_ASSERT_MATCHING_ENUM(mojom::BackgroundSyncError::MAX,
                              BACKGROUND_SYNC_STATUS_PERMISSION_DENIED);
 
-COMPILE_ASSERT_MATCHING_ENUM(mojom::BackgroundSyncNetworkState::ANY,
-                             SyncNetworkState::NETWORK_STATE_ANY);
-COMPILE_ASSERT_MATCHING_ENUM(mojom::BackgroundSyncNetworkState::AVOID_CELLULAR,
-                             SyncNetworkState::NETWORK_STATE_AVOID_CELLULAR);
-COMPILE_ASSERT_MATCHING_ENUM(mojom::BackgroundSyncNetworkState::ONLINE,
-                             SyncNetworkState::NETWORK_STATE_ONLINE);
-COMPILE_ASSERT_MATCHING_ENUM(mojom::BackgroundSyncNetworkState::MAX,
-                             SyncNetworkState::NETWORK_STATE_ONLINE);
-
 BackgroundSyncServiceImpl::~BackgroundSyncServiceImpl() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(background_sync_context_->background_sync_manager());

third_party/blink/public/platform/modules/background_sync/background_sync.mojom

@@ -4,16 +4,8 @@
 
 module blink.mojom;
 
-enum BackgroundSyncNetworkState {
-  ANY,
-  AVOID_CELLULAR,
-  ONLINE,
-  MAX=ONLINE
-};
-
 struct SyncRegistration {
   string tag = "";
-  BackgroundSyncNetworkState network_state = ONLINE;
 };
 
 enum BackgroundSyncError {

third_party/blink/renderer/modules/background_sync/sync_manager.cc

@@ -40,8 +40,6 @@ ScriptPromise SyncManager::registerFunction(ScriptState* script_state,
   mojom::blink::SyncRegistrationPtr sync_registration =
       mojom::blink::SyncRegistration::New();
   sync_registration->tag = tag;
-  sync_registration->network_state =
-      blink::mojom::BackgroundSyncNetworkState::ONLINE;
 
   GetBackgroundSyncServicePtr()->Register(
       std::move(sync_registration), registration_->RegistrationId(),