From 29bf0405e3f0871b659aa772f88e191815e2141d Mon Sep 17 00:00:00 2001 From: Kartik Hegde Date: Mon, 4 Mar 2024 19:51:21 +0000 Subject: [PATCH] traffic_counters: Remove all auto reset enabled code This code was originally added to support the toggle functionality for traffic counters. Since the toggle functionality is no longer supported, remove it from the UX and model. See bug for additional details. BUG=b:324745923 TEST=existing unit tests Change-Id: Ica2e50b93db3814ccfd5eae5a3cb043f8a1f8aa6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5284327 Reviewed-by: Steven Bennetts Reviewed-by: Ashley Prasad Reviewed-by: Alex Gough Reviewed-by: Artem Sumaneev Commit-Queue: Kartik Hegde Reviewed-by: Kyle Horimoto Cr-Commit-Position: refs/heads/main@{#1268015} --- .../traffic_counters_adapter.ts | 30 +--- .../net/traffic_counters_handler_unittest.cc | 3 - .../fake_cros_network_config_base.cc | 5 +- .../fake_cros_network_config_base.h | 5 +- .../settings_traffic_counters.html | 28 +--- .../settings_traffic_counters.ts | 57 +------- .../chromeos/fake_network_config_mojom.js | 16 +- .../settings_traffic_counters_test.ts | 105 +------------- .../network/network_metadata_store.cc | 13 -- .../network/network_metadata_store.h | 9 -- .../network_metadata_store_unittest.cc | 23 +-- .../network_config/cros_network_config.cc | 28 +--- .../network_config/cros_network_config.h | 5 +- .../cros_network_config_unittest.cc | 137 ++++-------------- .../public/cpp/fake_cros_network_config.h | 5 +- .../public/mojom/cros_network_config.mojom | 8 +- 16 files changed, 70 insertions(+), 407 deletions(-) diff --git a/ash/webui/common/resources/traffic_counters/traffic_counters_adapter.ts b/ash/webui/common/resources/traffic_counters/traffic_counters_adapter.ts index a03e5c68406bb9..c1425b81147fc1 100644 --- a/ash/webui/common/resources/traffic_counters/traffic_counters_adapter.ts +++ b/ash/webui/common/resources/traffic_counters/traffic_counters_adapter.ts @@ -25,7 +25,6 @@ export interface Network { counters: TrafficCounter[]; lastResetTime: Time|null; friendlyDate: string|null; - autoReset: boolean; userSpecifiedResetDay: number; } @@ -40,7 +39,6 @@ function createNetwork( counters: TrafficCounter[], lastResetTime: Time|null, friendlyDate: string|null, - autoReset: boolean, userSpecifiedResetDay: number): Network { return { guid: guid, @@ -49,7 +47,6 @@ function createNetwork( counters: counters, lastResetTime: lastResetTime, friendlyDate: friendlyDate, - autoReset: autoReset, userSpecifiedResetDay: userSpecifiedResetDay, }; } @@ -84,14 +81,11 @@ export class TrafficCountersAdapter { await this.requestLastResetTimeForNetwork(networkState.guid); const friendlyDate = await this.requestFriendlyDateForNetwork(networkState.guid); - const autoReset = - await this.requestEnableAutoResetBooleanForNetwork(networkState.guid); const userSpecifiedResetDay = await this.requestUserSpecifiedResetDayForNetwork(networkState.guid); networks.push(createNetwork( networkState.guid, networkState.name, networkState.type, - trafficCounters, lastResetTime, friendlyDate, autoReset, - userSpecifiedResetDay)); + trafficCounters, lastResetTime, friendlyDate, userSpecifiedResetDay)); } return networks; } @@ -142,19 +136,6 @@ export class TrafficCountersAdapter { null; } - /** - * Requests enable traffic counters auto reset boolean for the given network. - */ - async requestEnableAutoResetBooleanForNetwork( - guid: string): Promise { - const managedPropertiesPromise = - await this.networkConfig_.getManagedProperties(guid); - if (!managedPropertiesPromise || !managedPropertiesPromise.result) { - return false; - } - return managedPropertiesPromise.result.trafficCounterProperties.autoReset; - } - /** * Requests user specified reset day for the given network. */ @@ -171,11 +152,8 @@ export class TrafficCountersAdapter { /** * Sets values for auto reset. */ - async setTrafficCountersAutoResetForNetwork( - guid: string, - autoReset: boolean, - resetDay: UInt32Value|null): Promise { - await this.networkConfig_.setTrafficCountersAutoReset( - guid, autoReset, resetDay); + async setTrafficCountersResetDayForNetwork( + guid: string, resetDay: UInt32Value|null): Promise { + await this.networkConfig_.setTrafficCountersResetDay(guid, resetDay); } } diff --git a/chrome/browser/ash/net/traffic_counters_handler_unittest.cc b/chrome/browser/ash/net/traffic_counters_handler_unittest.cc index 02ba0338b54c14..ba90dd2c8678c1 100644 --- a/chrome/browser/ash/net/traffic_counters_handler_unittest.cc +++ b/chrome/browser/ash/net/traffic_counters_handler_unittest.cc @@ -162,9 +162,6 @@ class TrafficCountersHandlerTest : public ::testing::Test { base::Value(shill::kStateOnline)); helper_->profile_test()->AddService( NetworkProfileHandler::GetSharedProfilePath(), wifi_path_); - NetworkHandler::Get() - ->network_metadata_store() - ->SetEnableTrafficCountersAutoReset(wifi_guid_, false); task_environment_.RunUntilIdle(); } diff --git a/chrome/browser/ash/policy/remote_commands/fake_cros_network_config_base.cc b/chrome/browser/ash/policy/remote_commands/fake_cros_network_config_base.cc index 19cbb9f6cd2e33..f875c43f5f9422 100644 --- a/chrome/browser/ash/policy/remote_commands/fake_cros_network_config_base.cc +++ b/chrome/browser/ash/policy/remote_commands/fake_cros_network_config_base.cc @@ -136,11 +136,10 @@ void FakeCrosNetworkConfigBase::ResetTrafficCounters(const std::string& guid) { NOTREACHED(); } -void FakeCrosNetworkConfigBase::SetTrafficCountersAutoReset( +void FakeCrosNetworkConfigBase::SetTrafficCountersResetDay( const std::string& guid, - bool auto_reset, chromeos::network_config::mojom::UInt32ValuePtr day, - SetTrafficCountersAutoResetCallback callback) { + SetTrafficCountersResetDayCallback callback) { NOTREACHED(); } diff --git a/chrome/browser/ash/policy/remote_commands/fake_cros_network_config_base.h b/chrome/browser/ash/policy/remote_commands/fake_cros_network_config_base.h index 3a6542c170b50d..61fd14c27d9787 100644 --- a/chrome/browser/ash/policy/remote_commands/fake_cros_network_config_base.h +++ b/chrome/browser/ash/policy/remote_commands/fake_cros_network_config_base.h @@ -77,11 +77,10 @@ class FakeCrosNetworkConfigBase void RequestTrafficCounters(const std::string& guid, RequestTrafficCountersCallback callback) override; void ResetTrafficCounters(const std::string& guid) override; - void SetTrafficCountersAutoReset( + void SetTrafficCountersResetDay( const std::string& guid, - bool auto_reset, chromeos::network_config::mojom::UInt32ValuePtr day, - SetTrafficCountersAutoResetCallback callback) override; + SetTrafficCountersResetDayCallback callback) override; void CreateCustomApn(const std::string& network_guid, chromeos::network_config::mojom::ApnPropertiesPtr apn, CreateCustomApnCallback callback) override; diff --git a/chrome/browser/resources/ash/settings/internet_page/settings_traffic_counters.html b/chrome/browser/resources/ash/settings/internet_page/settings_traffic_counters.html index 4d7c350218df25..46df40bea0817a 100644 --- a/chrome/browser/resources/ash/settings/internet_page/settings_traffic_counters.html +++ b/chrome/browser/resources/ash/settings/internet_page/settings_traffic_counters.html @@ -31,32 +31,14 @@
-
\ No newline at end of file diff --git a/chrome/browser/resources/ash/settings/internet_page/settings_traffic_counters.ts b/chrome/browser/resources/ash/settings/internet_page/settings_traffic_counters.ts index 62b804532b6e14..b78c5cfcb9410f 100644 --- a/chrome/browser/resources/ash/settings/internet_page/settings_traffic_counters.ts +++ b/chrome/browser/resources/ash/settings/internet_page/settings_traffic_counters.ts @@ -92,21 +92,15 @@ export class SettingsTrafficCountersElement extends type: String, value: '', }, - /** Tracks whether auto reset is enabled. */ - autoReset_: { - type: Boolean, - value: false, - }, /** Tracks the user specified day of reset. Default is 1. */ resetDay_: { type: Number, - value: 1, + value: DEFAULT_RESET_DAY, }, }; } guid: string; - private autoReset_: boolean; private date_: string; private resetDay_: number; private trafficCountersAdapter_: TrafficCountersAdapter; @@ -130,7 +124,7 @@ export class SettingsTrafficCountersElement extends this.populateTrafficCountersAvailable_(); this.populateDate_(); this.populateDataUsageValue_(); - this.populateAutoResetValues_(); + this.populateUserSpecifiedResetDay_(); } /** @@ -217,31 +211,7 @@ export class SettingsTrafficCountersElement extends } /** - * Populates the auto reset enable and day values. - */ - private populateAutoResetValues_(): void { - this.populateEnableAutoResetBoolean_(); - this.populateUserSpecifiedResetDay_(); - } - - /** - * Determines whether auto reset is enabled. - */ - private async populateEnableAutoResetBoolean_(): Promise { - const result = await this.populateEnableAutoResetBooleanHelper_(); - this.autoReset_ = result; - } - - /** - * Gathers auto reset enable information. - */ - private async populateEnableAutoResetBooleanHelper_(): Promise { - const network = await this.getNetworkIfAvailable_(); - return network ? network.autoReset : false; - } - - /** - * Determines the auto reset day. + * Determines the reset day. */ private async populateUserSpecifiedResetDay_(): Promise { const result = await this.populateUserSpecifiedResetDayHelper_(); @@ -249,34 +219,19 @@ export class SettingsTrafficCountersElement extends } /** - * Gathers the auto reset day information. + * Gathers the reset day information (helper). */ private async populateUserSpecifiedResetDayHelper_(): Promise { const network = await this.getNetworkIfAvailable_(); return network ? network.userSpecifiedResetDay : DEFAULT_RESET_DAY; } - /** - * Handles the auto reset toggle changes. - */ - private onAutoDataUsageResetToggle_(): void { - this.autoReset_ = !this.autoReset_; - this.resetDay_ = 1; - const day = this.autoReset_ ? {value: this.resetDay_} : null; - this.trafficCountersAdapter_.setTrafficCountersAutoResetForNetwork( - this.guid, this.autoReset_, day); - this.load(); - } - /** * Handles day of reset changes. */ private onResetDaySelected_(): void { - if (!this.autoReset_) { - return; - } - this.trafficCountersAdapter_.setTrafficCountersAutoResetForNetwork( - this.guid, this.autoReset_, {value: this.resetDay_}); + this.trafficCountersAdapter_.setTrafficCountersResetDayForNetwork( + this.guid, {value: this.resetDay_}); this.load(); } } diff --git a/chrome/test/data/webui/chromeos/fake_network_config_mojom.js b/chrome/test/data/webui/chromeos/fake_network_config_mojom.js index 5ca2d7a9576abf..65abb287d3f4fd 100644 --- a/chrome/test/data/webui/chromeos/fake_network_config_mojom.js +++ b/chrome/test/data/webui/chromeos/fake_network_config_mojom.js @@ -87,9 +87,6 @@ export class FakeNetworkConfig { /** @private {!Map>} */ this.trafficCountersMap_ = new Map(); - /** @private {!Map>} */ - this.autoResetValuesMap_ = new Map(); - /** @private {!number} */ this.apnIdCounter_ = 0; @@ -151,7 +148,7 @@ export class FakeNetworkConfig { 'setProperties', 'setCellularSimState', 'selectCellularMobileNetwork', 'startConnect', 'startDisconnect', 'configureNetwork', 'forgetNetwork', 'getAlwaysOnVpn', 'getSupportedVpnTypes', 'requestTrafficCounters', - 'resetTrafficCounters', 'setTrafficCountersAutoReset', 'removeCustomApn', + 'resetTrafficCounters', 'setTrafficCountersResetDay', 'removeCustomApn', 'createCustomApn', 'createExclusivelyEnabledCustomApn', 'modifyCustomApn'] .forEach((methodName) => { this.resolverMap_.set(methodName, new PromiseResolver()); @@ -783,17 +780,15 @@ export class FakeNetworkConfig { /** * @param {string} guid - * @param {boolean} autoReset * @param {?UInt32Value} resetDay */ - setAutoResetValues_(guid, autoReset, resetDay) { + setResetDay_(guid, resetDay) { const network = this.networkStates_.find(state => { return state.guid === guid; }); assert(!!network, 'Network not found: ' + guid); const managed = this.managedProperties_.get(guid); if (managed) { - managed.trafficCounterProperties.autoReset = autoReset; managed.trafficCounterProperties.userSpecifiedResetDay = resetDay ? resetDay.value : 1; } @@ -802,13 +797,12 @@ export class FakeNetworkConfig { /** * @param {string} guid - * @param {boolean} autoReset * @param {?UInt32Value} resetDay */ - setTrafficCountersAutoReset(guid, autoReset, resetDay) { + setTrafficCountersResetDay(guid, resetDay) { return new Promise(resolve => { - this.methodCalled('setTrafficCountersAutoReset'); - this.setAutoResetValues_(guid, autoReset, resetDay); + this.methodCalled('setTrafficCountersResetDay'); + this.setResetDay_(guid, resetDay); resolve(true); }); } diff --git a/chrome/test/data/webui/settings/chromeos/internet_page/settings_traffic_counters_test.ts b/chrome/test/data/webui/settings/chromeos/internet_page/settings_traffic_counters_test.ts index e037af20a73d41..fd3a882cbb6887 100644 --- a/chrome/test/data/webui/settings/chromeos/internet_page/settings_traffic_counters_test.ts +++ b/chrome/test/data/webui/settings/chromeos/internet_page/settings_traffic_counters_test.ts @@ -11,7 +11,7 @@ import {TrafficCounter, TrafficCounterSource} from 'chrome://resources/mojo/chro import {ConnectionStateType, NetworkType} from 'chrome://resources/mojo/chromeos/services/network_config/public/mojom/network_types.mojom-webui.js'; import {Time} from 'chrome://resources/mojo/mojo/public/mojom/base/time.mojom-webui.js'; import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js'; -import {assertEquals, assertNull, assertTrue} from 'chrome://webui-test/chai_assert.js'; +import {assertEquals, assertTrue} from 'chrome://webui-test/chai_assert.js'; import {FakeNetworkConfig} from 'chrome://webui-test/chromeos/fake_network_config_mojom.js'; import {flushTasks} from 'chrome://webui-test/polymer_test_util.js'; @@ -116,43 +116,6 @@ suite('', () => { return resetDataUsageButton; } - function getAutoDataUsageResetLabel(): string { - const autoDataUsageResetLabelDiv = - settingsTrafficCounters.shadowRoot!.querySelector( - '#autoDataUsageResetLabel'); - assertTrue(!!autoDataUsageResetLabelDiv); - return autoDataUsageResetLabelDiv.textContent!.trim(); - } - - function getAutoDataUsageResetSubLabel(): string { - const autoDataUsageResetSubLabelDiv = - settingsTrafficCounters.shadowRoot!.querySelector( - '#autoDataUsageResetSubLabel'); - assertTrue(!!autoDataUsageResetSubLabelDiv); - return autoDataUsageResetSubLabelDiv.textContent!.trim(); - } - - function getDaySelectionLabel(): string { - const daySelectionLabelDiv = - settingsTrafficCounters.shadowRoot!.querySelector('#daySelectionLabel'); - assertTrue(!!daySelectionLabelDiv); - return daySelectionLabelDiv.textContent!.trim(); - } - - function getAutoDataUsageResetToggle(): HTMLButtonElement { - const autoDataUsageResetToggle = - settingsTrafficCounters.shadowRoot!.querySelector( - '#autoDataUsageResetToggle'); - assertTrue(!!autoDataUsageResetToggle); - return autoDataUsageResetToggle; - } - - function ensureDaySelectionInputIsNotPresent(): void { - const daySelectionInput = - settingsTrafficCounters.shadowRoot!.querySelector('#daySelectionInput'); - assertNull(daySelectionInput); - } - function getDaySelectionInput(): HTMLInputElement { const daySelectionInput = settingsTrafficCounters.shadowRoot!.querySelector( @@ -187,7 +150,6 @@ suite('', () => { NetworkType.kCellular, 'cellular_guid', 'cellular'); managedProperties.connectionState = ConnectionStateType.kConnected; managedProperties.connectable = true; - managedProperties.trafficCounterProperties.autoReset = true; managedProperties.trafficCounterProperties.userSpecifiedResetDay = 31; networkConfigRemote.setManagedPropertiesForTest(managedProperties); await flushTasks(); @@ -220,7 +182,6 @@ suite('', () => { FAKE_INITIAL_LAST_RESET_TIME; managedProperties.trafficCounterProperties.friendlyDate = FAKE_INITIAL_FRIENDLY_DATE; - managedProperties.trafficCounterProperties.autoReset = true; managedProperties.trafficCounterProperties.userSpecifiedResetDay = 31; networkConfigRemote.setManagedPropertiesForTest(managedProperties); await flushTasks(); @@ -241,19 +202,6 @@ suite('', () => { assertEquals( settingsTrafficCounters.i18n('TrafficCountersDataUsageResetLabel'), getResetDataUsageLabel()); - assertEquals( - settingsTrafficCounters.i18n( - 'TrafficCountersDataUsageEnableAutoResetLabel'), - getAutoDataUsageResetLabel()); - assertEquals( - settingsTrafficCounters.i18n( - 'TrafficCountersDataUsageEnableAutoResetSublabel'), - getAutoDataUsageResetSubLabel()); - assertEquals( - settingsTrafficCounters.i18n( - 'TrafficCountersDataUsageAutoResetDayOfMonthLabel'), - getDaySelectionLabel()); - getAutoDataUsageResetToggle(); assertEquals(31, getDaySelectionInput().value); // Simulate a reset by updating the last reset time for the cellular @@ -278,55 +226,4 @@ suite('', () => { settingsTrafficCounters.i18n('TrafficCountersDataUsageResetLabel'), getResetDataUsageLabel()); }); - - test('Enable traffic counters auto reset', async () => { - // Disable auto reset initially. - const managedProperties = OncMojo.getDefaultManagedProperties( - NetworkType.kCellular, 'cellular_guid', 'cellular'); - managedProperties.connectionState = ConnectionStateType.kConnected; - managedProperties.connectable = true; - managedProperties.trafficCounterProperties.autoReset = false; - managedProperties.trafficCounterProperties.userSpecifiedResetDay = 0; - managedProperties.trafficCounterProperties.lastResetTime = - FAKE_INITIAL_LAST_RESET_TIME; - managedProperties.trafficCounterProperties.friendlyDate = - FAKE_INITIAL_FRIENDLY_DATE; - networkConfigRemote.setManagedPropertiesForTest(managedProperties); - await flushTasks(); - - // Set traffic counters. - // A bigint primitive is created by appending n to the end of an integer - // literal - networkConfigRemote.setTrafficCountersForTest( - 'cellular_guid', generateTrafficCounters(100n, 100n)); - await flushTasks(); - - // Load the settings traffic counters HTML for cellular_guid. - settingsTrafficCounters.guid = 'cellular_guid'; - settingsTrafficCounters.load(); - await flushTasks(); - ensureDaySelectionInputIsNotPresent(); - - // Enable auto reset. - getAutoDataUsageResetToggle().click(); - await flushTasks(); - - // Verify that the correct day selection label is shown. - assertEquals( - settingsTrafficCounters.i18n( - 'TrafficCountersDataUsageAutoResetDayOfMonthLabel'), - getDaySelectionLabel()); - // Verify that the correct user specified day is shown. - assertEquals(1, getDaySelectionInput().value); - - // Change the reset day to a valid value. - getDaySelectionInput().value = '15'; - await flushTasks(); - assertEquals('15', getDaySelectionInput().value); - - // Disable auto reset again. - getAutoDataUsageResetToggle().click(); - await flushTasks(); - ensureDaySelectionInputIsNotPresent(); - }); }); diff --git a/chromeos/ash/components/network/network_metadata_store.cc b/chromeos/ash/components/network/network_metadata_store.cc index 47fc51e09ba40a..bcb5e11c5f27d3 100644 --- a/chromeos/ash/components/network/network_metadata_store.cc +++ b/chromeos/ash/components/network/network_metadata_store.cc @@ -50,8 +50,6 @@ const char kCustomApnList[] = "custom_apn_list"; const char kCustomApnListV2[] = "custom_apn_list_v2"; const char kHasFixedHiddenNetworks[] = "metadata_store.has_fixed_hidden_networks"; -const char kEnableTrafficCountersAutoReset[] = - "enable_traffic_counters_auto_reset"; const char kDayOfTrafficCountersAutoReset[] = "day_of_traffic_counters_auto_reset"; const char kUserTextMessageSuppressionState[] = @@ -557,12 +555,6 @@ const base::Value::List* NetworkMetadataStore::GetPreRevampCustomApnList( return nullptr; } -void NetworkMetadataStore::SetEnableTrafficCountersAutoReset( - const std::string& network_guid, - bool enable) { - SetPref(network_guid, kEnableTrafficCountersAutoReset, base::Value(enable)); -} - void NetworkMetadataStore::SetDayOfTrafficCountersAutoReset( const std::string& network_guid, const std::optional& day) { @@ -570,11 +562,6 @@ void NetworkMetadataStore::SetDayOfTrafficCountersAutoReset( SetPref(network_guid, kDayOfTrafficCountersAutoReset, std::move(value)); } -const base::Value* NetworkMetadataStore::GetEnableTrafficCountersAutoReset( - const std::string& network_guid) { - return GetPref(network_guid, kEnableTrafficCountersAutoReset); -} - const base::Value* NetworkMetadataStore::GetDayOfTrafficCountersAutoReset( const std::string& network_guid) { return GetPref(network_guid, kDayOfTrafficCountersAutoReset); diff --git a/chromeos/ash/components/network/network_metadata_store.h b/chromeos/ash/components/network/network_metadata_store.h index dce11484a703a1..204eca41f9147c 100644 --- a/chromeos/ash/components/network/network_metadata_store.h +++ b/chromeos/ash/components/network/network_metadata_store.h @@ -132,20 +132,11 @@ class COMPONENT_EXPORT(CHROMEOS_NETWORK) NetworkMetadataStore // marks networks that were added in OOBE to the user's list. void OwnSharedNetworksOnFirstUserLogin(); - // Sets whether traffic counters should be automatically reset. - void SetEnableTrafficCountersAutoReset(const std::string& network_guid, - bool enable); - // Sets the day of the month on which traffic counters are automatically // reset. void SetDayOfTrafficCountersAutoReset(const std::string& network_guid, const std::optional& day); - // Returns whether traffic counters should be automatically reset. Returns - // nullptr if no pref exists for |network_guid|. - const base::Value* GetEnableTrafficCountersAutoReset( - const std::string& network_guid); - // Returns the day of the month on which traffic counters are automatically // reset. Returns nullptr if no pref exists for |network_guid|. const base::Value* GetDayOfTrafficCountersAutoReset( diff --git a/chromeos/ash/components/network/network_metadata_store_unittest.cc b/chromeos/ash/components/network/network_metadata_store_unittest.cc index 42ad3d835353e7..56a58afb710b56 100644 --- a/chromeos/ash/components/network/network_metadata_store_unittest.cc +++ b/chromeos/ash/components/network/network_metadata_store_unittest.cc @@ -681,28 +681,7 @@ TEST_F(NetworkMetadataStoreTest, LogHiddenNetworks) { /*sample=*/false, /*expected_count=*/1); } -TEST_F(NetworkMetadataStoreTest, EnableAndDisableTrafficCountersAutoReset) { - std::string service_path = ConfigureService(kConfigWifi0Connectable); - const base::Value* value = - metadata_store()->GetEnableTrafficCountersAutoReset(kGuid); - EXPECT_EQ(nullptr, value); - - metadata_store()->SetEnableTrafficCountersAutoReset(kGuid, /*enable=*/true); - base::RunLoop().RunUntilIdle(); - - value = metadata_store()->GetEnableTrafficCountersAutoReset(kGuid); - ASSERT_TRUE(value && value->is_bool()); - EXPECT_TRUE(value->GetBool()); - - metadata_store()->SetEnableTrafficCountersAutoReset(kGuid, /*enable=*/false); - base::RunLoop().RunUntilIdle(); - - value = metadata_store()->GetEnableTrafficCountersAutoReset(kGuid); - ASSERT_TRUE(value && value->is_bool()); - EXPECT_FALSE(value->GetBool()); -} - -TEST_F(NetworkMetadataStoreTest, SetTrafficCountersAutoResetDay) { +TEST_F(NetworkMetadataStoreTest, SetTrafficCountersResetDay) { std::string service_path = ConfigureService(kConfigWifi0Connectable); const base::Value* value = metadata_store()->GetDayOfTrafficCountersAutoReset(kGuid); diff --git a/chromeos/ash/services/network_config/cros_network_config.cc b/chromeos/ash/services/network_config/cros_network_config.cc index 3e332c9b7b36f5..f440e3d531ee98 100644 --- a/chromeos/ash/services/network_config/cros_network_config.cc +++ b/chromeos/ash/services/network_config/cros_network_config.cc @@ -1845,14 +1845,6 @@ mojom::ManagedPropertiesPtr ManagedPropertiesToMojo( traffic_counter_properties->friendly_date = std::nullopt; } - const base::Value* auto_reset = - NetworkHandler::IsInitialized() - ? NetworkHandler::Get() - ->network_metadata_store() - ->GetEnableTrafficCountersAutoReset(result->guid) - : nullptr; - traffic_counter_properties->auto_reset = - auto_reset && auto_reset->is_bool() ? auto_reset->GetBool() : false; const base::Value* user_specified_reset_day = NetworkHandler::IsInitialized() ? NetworkHandler::Get() @@ -3501,32 +3493,22 @@ void CrosNetworkConfig::ResetTrafficCounters(const std::string& guid) { network_state_handler_->ResetTrafficCounters(service_path); } -void CrosNetworkConfig::SetTrafficCountersAutoReset( +void CrosNetworkConfig::SetTrafficCountersResetDay( const std::string& guid, - bool auto_reset, mojom::UInt32ValuePtr day, - SetTrafficCountersAutoResetCallback callback) { - if (day && !auto_reset) { - NET_LOG(ERROR) << "Failed to set auto reset day for " << guid - << ": auto reset must be enabled."; - std::move(callback).Run(/*success=*/false); - return; - } - if (!day && auto_reset) { - NET_LOG(ERROR) << "Failed to enable auto reset for " << guid << ": a valid " + SetTrafficCountersResetDayCallback callback) { + if (!day) { + NET_LOG(ERROR) << "Failed to set reset day for " << guid << ": a valid " << "day between 1 and 31 (inclusive) must be provided."; std::move(callback).Run(/*success=*/false); return; } if (day && (day->value < 1 || day->value > 31)) { - NET_LOG(ERROR) << "Failed to set auto reset day " << day->value << " for " + NET_LOG(ERROR) << "Failed to set reset day " << day->value << " for " << guid << ": day must be between 1 and 31 (inclusive)"; std::move(callback).Run(/*success=*/false); return; } - NetworkHandler::Get() - ->network_metadata_store() - ->SetEnableTrafficCountersAutoReset(guid, auto_reset); NetworkHandler::Get() ->network_metadata_store() ->SetDayOfTrafficCountersAutoReset( diff --git a/chromeos/ash/services/network_config/cros_network_config.h b/chromeos/ash/services/network_config/cros_network_config.h index 88edab657c02c6..1748b22eea4e6f 100644 --- a/chromeos/ash/services/network_config/cros_network_config.h +++ b/chromeos/ash/services/network_config/cros_network_config.h @@ -114,11 +114,10 @@ class CrosNetworkConfig void RequestTrafficCounters(const std::string& guid, RequestTrafficCountersCallback callback) override; void ResetTrafficCounters(const std::string& guid) override; - void SetTrafficCountersAutoReset( + void SetTrafficCountersResetDay( const std::string& guid, - bool auto_reset, chromeos::network_config::mojom::UInt32ValuePtr day, - SetTrafficCountersAutoResetCallback callback) override; + SetTrafficCountersResetDayCallback callback) override; void CreateCustomApn(const std::string& network_guid, chromeos::network_config::mojom::ApnPropertiesPtr apn, CreateCustomApnCallback callback) override; diff --git a/chromeos/ash/services/network_config/cros_network_config_unittest.cc b/chromeos/ash/services/network_config/cros_network_config_unittest.cc index d36931d0c65242..834db02f1f5a63 100644 --- a/chromeos/ash/services/network_config/cros_network_config_unittest.cc +++ b/chromeos/ash/services/network_config/cros_network_config_unittest.cc @@ -879,34 +879,22 @@ class CrosNetworkConfigTest : public testing::Test { run_loop.Run(); } - void SetTrafficCountersAutoResetAndCompare(const std::string& guid, - bool auto_reset, - mojom::UInt32ValuePtr day, - bool expected_success, - base::Value* expected_auto_reset, - base::Value* expected_reset_day) { + void SetTrafficCountersResetDayAndCompare(const std::string& guid, + mojom::UInt32ValuePtr day, + bool expected_success, + base::Value* expected_reset_day) { base::RunLoop run_loop; - cros_network_config()->SetTrafficCountersAutoReset( - guid, auto_reset, day ? std::move(day) : nullptr, + cros_network_config()->SetTrafficCountersResetDay( + guid, day ? std::move(day) : nullptr, base::BindOnce( [](const std::string* const guid, bool* expected_success, - base::Value* expected_auto_reset, base::Value* expected_reset_day, NetworkMetadataStore* network_metadata_store, base::OnceClosure quit_closure, bool success) { EXPECT_EQ(*expected_success, success); - const base::Value* actual_auto_reset = - network_metadata_store->GetEnableTrafficCountersAutoReset( - *guid); const base::Value* actual_reset_day = network_metadata_store->GetDayOfTrafficCountersAutoReset( *guid); - if (expected_auto_reset) { - EXPECT_TRUE(actual_auto_reset); - EXPECT_EQ(*expected_auto_reset, *actual_auto_reset); - } else { - EXPECT_EQ(actual_auto_reset, nullptr); - } if (expected_reset_day) { EXPECT_TRUE(actual_reset_day); EXPECT_EQ(*expected_reset_day, *actual_reset_day); @@ -915,7 +903,7 @@ class CrosNetworkConfigTest : public testing::Test { } std::move(quit_closure).Run(); }, - &guid, &expected_success, expected_auto_reset, expected_reset_day, + &guid, &expected_success, expected_reset_day, network_metadata_store(), run_loop.QuitClosure())); run_loop.Run(); } @@ -1815,28 +1803,25 @@ TEST_F(CrosNetworkConfigTest, GetDeviceStateListNoVpnServicesAndVpnProhibited) { // translated as strings and not enum values (See ManagedProperties definition // in cros_network_config.mojom for details). TEST_F(CrosNetworkConfigTest, GetManagedProperties) { - SetTrafficCountersAutoResetAndCompare("eth_guid", /*auto_reset=*/true, - /*day=*/mojom::UInt32Value::New(32), - /*expected_success=*/false, - /*expected_auto_reset=*/nullptr, - /*expected_reset_day=*/nullptr); + SetTrafficCountersResetDayAndCompare("eth_guid", + /*day=*/mojom::UInt32Value::New(32), + /*expected_success=*/false, + /*expected_reset_day=*/nullptr); mojom::ManagedPropertiesPtr properties = GetManagedProperties("eth_guid"); ASSERT_TRUE(properties); EXPECT_EQ("eth_guid", properties->guid); EXPECT_EQ(mojom::NetworkType::kEthernet, properties->type); EXPECT_EQ(mojom::ConnectionStateType::kOnline, properties->connection_state); ASSERT_TRUE(properties->traffic_counter_properties); - EXPECT_FALSE(properties->traffic_counter_properties->auto_reset); EXPECT_EQ(static_cast(1), properties->traffic_counter_properties->user_specified_reset_day); EXPECT_FALSE(properties->traffic_counter_properties->last_reset_time); - base::Value expected_auto_reset(true); base::Value expected_reset_day(2); - SetTrafficCountersAutoResetAndCompare( - "wifi1_guid", /*auto_reset=*/true, - /*day=*/mojom::UInt32Value::New(2), - /*expected_success=*/true, &expected_auto_reset, &expected_reset_day); + SetTrafficCountersResetDayAndCompare("wifi1_guid", + /*day=*/mojom::UInt32Value::New(2), + /*expected_success=*/true, + &expected_reset_day); properties = GetManagedProperties("wifi1_guid"); ASSERT_TRUE(properties); EXPECT_EQ("wifi1_guid", properties->guid); @@ -1854,15 +1839,13 @@ TEST_F(CrosNetworkConfigTest, GetManagedProperties) { properties->traffic_counter_properties->last_reset_time ->ToDeltaSinceWindowsEpoch() .InMilliseconds()); - EXPECT_TRUE(properties->traffic_counter_properties->auto_reset); EXPECT_EQ(static_cast(2), properties->traffic_counter_properties->user_specified_reset_day); - SetTrafficCountersAutoResetAndCompare("wifi2_guid", /*auto_reset=*/true, - /*day=*/nullptr, - /*expected_success=*/false, - /*expected_auto_reset=*/nullptr, - /*expected_reset_day=*/nullptr); + SetTrafficCountersResetDayAndCompare("wifi2_guid", + /*day=*/nullptr, + /*expected_success=*/false, + /*expected_reset_day=*/nullptr); properties = GetManagedProperties("wifi2_guid"); ASSERT_TRUE(properties); EXPECT_EQ("wifi2_guid", properties->guid); @@ -1877,42 +1860,6 @@ TEST_F(CrosNetworkConfigTest, GetManagedProperties) { EXPECT_EQ(100, wifi->signal_strength); EXPECT_EQ(mojom::OncSource::kUserPolicy, properties->source); EXPECT_FALSE(properties->type_properties->get_wifi()->is_syncable); - EXPECT_FALSE(properties->traffic_counter_properties->auto_reset); - EXPECT_EQ(static_cast(1), - properties->traffic_counter_properties->user_specified_reset_day); - - SetTrafficCountersAutoResetAndCompare("wifi3_guid", /*auto_reset=*/false, - /*day=*/mojom::UInt32Value::New(2), - /*expected_success=*/false, - /*expected_auto_reset=*/nullptr, - /*expected_reset_day=*/nullptr); - properties = GetManagedProperties("wifi3_guid"); - ASSERT_TRUE(properties); - EXPECT_EQ("wifi3_guid", properties->guid); - EXPECT_EQ(mojom::NetworkType::kWiFi, properties->type); - EXPECT_EQ(mojom::ConnectionStateType::kNotConnected, - properties->connection_state); - EXPECT_EQ(mojom::OncSource::kDevice, properties->source); - EXPECT_FALSE(properties->type_properties->get_wifi()->is_syncable); - EXPECT_FALSE(properties->traffic_counter_properties->auto_reset); - EXPECT_EQ(static_cast(1), - properties->traffic_counter_properties->user_specified_reset_day); - - expected_auto_reset = base::Value(false); - expected_reset_day = base::Value(); - SetTrafficCountersAutoResetAndCompare( - "wifi4_guid", /*auto_reset=*/false, - /*day=*/nullptr, - /*expected_success=*/true, &expected_auto_reset, &expected_reset_day); - properties = GetManagedProperties("wifi4_guid"); - ASSERT_TRUE(properties); - EXPECT_EQ("wifi4_guid", properties->guid); - EXPECT_EQ(mojom::NetworkType::kWiFi, properties->type); - EXPECT_EQ(mojom::ConnectionStateType::kNotConnected, - properties->connection_state); - EXPECT_EQ(mojom::OncSource::kUser, properties->source); - EXPECT_TRUE(properties->type_properties->get_wifi()->is_syncable); - EXPECT_FALSE(properties->traffic_counter_properties->auto_reset); EXPECT_EQ(static_cast(1), properties->traffic_counter_properties->user_specified_reset_day); @@ -4526,41 +4473,21 @@ TEST_F(CrosNetworkConfigTest, GetSupportedVpnTypes) { helper()->manager_test()->SetShouldReturnNullProperties(false); } -TEST_F(CrosNetworkConfigTest, SetAutoReset) { - SetTrafficCountersAutoResetAndCompare("wifi1_guid", /*auto_reset=*/true, - /*day=*/mojom::UInt32Value::New(32), - /*expected_success=*/false, - /*expected_auto_reset=*/nullptr, - /*expected_reset_day=*/nullptr); - base::Value expected_auto_reset(true); +TEST_F(CrosNetworkConfigTest, SetResetDay) { + SetTrafficCountersResetDayAndCompare("wifi1_guid", + /*day=*/mojom::UInt32Value::New(32), + /*expected_success=*/false, + /*expected_reset_day=*/nullptr); base::Value expected_reset_day(2); - SetTrafficCountersAutoResetAndCompare( - "wifi1_guid", /*auto_reset=*/true, - /*day=*/mojom::UInt32Value::New(2), - /*expected_success=*/true, &expected_auto_reset, &expected_reset_day); - // Auto reset prefs remains unchanged from last successful call. - SetTrafficCountersAutoResetAndCompare( - "wifi1_guid", /*auto_reset=*/true, - /*day=*/mojom::UInt32Value::New(0), - /*expected_success=*/false, &expected_auto_reset, &expected_reset_day); - expected_auto_reset = base::Value(false); - expected_reset_day = base::Value(); - SetTrafficCountersAutoResetAndCompare( - "wifi1_guid", /*auto_reset=*/false, - /*day=*/nullptr, - /*expected_success=*/true, - /*expected_auto_reset=*/&expected_auto_reset, - /*expected_reset_day=*/&expected_reset_day); - // Auto reset prefs remains unchanged from last successful call. - SetTrafficCountersAutoResetAndCompare( - "wifi1_guid", /*auto_reset=*/false, - /*day=*/mojom::UInt32Value::New(10), - /*expected_success=*/false, &expected_auto_reset, &expected_reset_day); + SetTrafficCountersResetDayAndCompare("wifi1_guid", + /*day=*/mojom::UInt32Value::New(2), + /*expected_success=*/true, + &expected_reset_day); // Auto reset prefs remains unchanged from last successful call. - SetTrafficCountersAutoResetAndCompare( - "wifi1_guid", /*auto_reset=*/true, - /*day=*/nullptr, - /*expected_success=*/false, &expected_auto_reset, &expected_reset_day); + SetTrafficCountersResetDayAndCompare("wifi1_guid", + /*day=*/mojom::UInt32Value::New(0), + /*expected_success=*/false, + &expected_reset_day); } // Make sure calling shutdown before cros_network_config destruction doesn't diff --git a/chromeos/services/network_config/public/cpp/fake_cros_network_config.h b/chromeos/services/network_config/public/cpp/fake_cros_network_config.h index cc16378d1818ed..b37cfd7d74f716 100644 --- a/chromeos/services/network_config/public/cpp/fake_cros_network_config.h +++ b/chromeos/services/network_config/public/cpp/fake_cros_network_config.h @@ -80,11 +80,10 @@ class FakeCrosNetworkConfig : public mojom::CrosNetworkConfig { const std::string& guid, RequestTrafficCountersCallback callback) override {} void ResetTrafficCounters(const std::string& guid) override {} - void SetTrafficCountersAutoReset( + void SetTrafficCountersResetDay( const std::string& guid, - bool auto_reset, mojom::UInt32ValuePtr day, - SetTrafficCountersAutoResetCallback callback) override {} + SetTrafficCountersResetDayCallback callback) override {} void CreateCustomApn(const std::string& network_guid, chromeos::network_config::mojom::ApnPropertiesPtr apn, CreateCustomApnCallback callback) override; diff --git a/chromeos/services/network_config/public/mojom/cros_network_config.mojom b/chromeos/services/network_config/public/mojom/cros_network_config.mojom index 7fabca17bf1615..0ee9313010bb92 100644 --- a/chromeos/services/network_config/public/mojom/cros_network_config.mojom +++ b/chromeos/services/network_config/public/mojom/cros_network_config.mojom @@ -1330,11 +1330,9 @@ interface CrosNetworkConfig { // Resets traffic counters for a service with ID |guid|. ResetTrafficCounters(string guid); - // Sets whether traffic counters should be automatically reset and the day of - // the month on which traffic counters are automatically reset. In order for - // |day| to be valid, it must be within the limit 1 <= |day| <= 31. - SetTrafficCountersAutoReset(string guid, bool auto_reset, UInt32Value? day) => - (bool success); + // Sets the traffic counters reset day. In order for |day| to be valid, it + // must be within the limit 1 <= |day| <= 31. + SetTrafficCountersResetDay(string guid, UInt32Value? day) => (bool success); // Creates a new APN for the cellular network with ID |network_guid|, and // appends it to |custom_apn_list|. Should only be called when the APN