Add TrimRemoteSpecificsForCaching
- Adds interface for trimming supported fields from EntitySpecifics.
By default returns empty EntitySpecifics proto.
- Adds password implementation of the function clearing all currently
supported fields.
Bug: 1296159
Change-Id: I416c78f4422762e2cad1030ca78838eb2e5c59dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3507175
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Andrii Natiahlyi <natiahlyi@google.com>
Cr-Commit-Position: refs/heads/main@{#982207}
diff --git a/components/password_manager/core/browser/sync/password_proto_utils.cc b/components/password_manager/core/browser/sync/password_proto_utils.cc
index fbcf7a5f..a9083f3 100644
--- a/components/password_manager/core/browser/sync/password_proto_utils.cc
+++ b/components/password_manager/core/browser/sync/password_proto_utils.cc
@@ -102,12 +102,44 @@
return form_issues;
}
+sync_pb::PasswordSpecificsData TrimPasswordSpecificsDataForCaching(
+ const sync_pb::PasswordSpecificsData& password_specifics_data) {
+ sync_pb::PasswordSpecificsData trimmed_password_data =
+ sync_pb::PasswordSpecificsData(password_specifics_data);
+ trimmed_password_data.clear_scheme();
+ trimmed_password_data.clear_signon_realm();
+ trimmed_password_data.clear_origin();
+ trimmed_password_data.clear_action();
+ trimmed_password_data.clear_username_element();
+ trimmed_password_data.clear_username_value();
+ trimmed_password_data.clear_password_element();
+ trimmed_password_data.clear_password_value();
+ trimmed_password_data.clear_date_created();
+ trimmed_password_data.clear_blacklisted();
+ trimmed_password_data.clear_type();
+ trimmed_password_data.clear_times_used();
+ trimmed_password_data.clear_display_name();
+ trimmed_password_data.clear_avatar_url();
+ trimmed_password_data.clear_federation_url();
+ trimmed_password_data.clear_date_last_used();
+ trimmed_password_data.clear_password_issues();
+ trimmed_password_data.clear_date_password_modified_windows_epoch_micros();
+ return trimmed_password_data;
+}
+
sync_pb::PasswordSpecifics SpecificsFromPassword(
const PasswordForm& password_form) {
sync_pb::PasswordSpecifics specifics;
*specifics.mutable_client_only_encrypted_data() =
SpecificsDataFromPassword(password_form);
+ // WARNING: if you are adding support for new `PasswordSpecificsData` fields,
+ // you need to update following functions accordingly:
+ // `TrimPasswordSpecificsDataForCaching`
+ // `TrimRemoteSpecificsForCachingPreservesOnlyUnknownFields`
+ DCHECK_EQ(0u, TrimPasswordSpecificsDataForCaching(
+ specifics.client_only_encrypted_data())
+ .ByteSizeLong());
return specifics;
}
diff --git a/components/password_manager/core/browser/sync/password_proto_utils.h b/components/password_manager/core/browser/sync/password_proto_utils.h
index 9d69c14..ef32d0f 100644
--- a/components/password_manager/core/browser/sync/password_proto_utils.h
+++ b/components/password_manager/core/browser/sync/password_proto_utils.h
@@ -57,6 +57,11 @@
std::vector<PasswordForm> PasswordVectorFromListResult(
const sync_pb::ListPasswordsResult& list_result);
+// Returns a copy of |password_specifics_data| with cleared supported fields
+// that don't need to be preserved in EntityMetadata cache.
+sync_pb::PasswordSpecificsData TrimPasswordSpecificsDataForCaching(
+ const sync_pb::PasswordSpecificsData& password_specifics_data);
+
} // namespace password_manager
#endif // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_SYNC_PASSWORD_PROTO_UTILS_H_
diff --git a/components/password_manager/core/browser/sync/password_sync_bridge.cc b/components/password_manager/core/browser/sync/password_sync_bridge.cc
index 4d958ea..6a83eee8 100644
--- a/components/password_manager/core/browser/sync/password_sync_bridge.cc
+++ b/components/password_manager/core/browser/sync/password_sync_bridge.cc
@@ -843,6 +843,17 @@
sync_enabled_or_disabled_cb_.Run();
}
+sync_pb::EntitySpecifics PasswordSyncBridge::TrimRemoteSpecificsForCaching(
+ const sync_pb::EntitySpecifics& entity_specifics) {
+ DCHECK(entity_specifics.has_password());
+ sync_pb::EntitySpecifics trimmed_entity_specifics;
+ *trimmed_entity_specifics.mutable_password()
+ ->mutable_client_only_encrypted_data() =
+ TrimPasswordSpecificsDataForCaching(
+ entity_specifics.password().client_only_encrypted_data());
+ return trimmed_entity_specifics;
+}
+
std::set<FormPrimaryKey> PasswordSyncBridge::GetUnsyncedPasswordsStorageKeys() {
std::set<FormPrimaryKey> storage_keys;
DCHECK(password_store_sync_);
diff --git a/components/password_manager/core/browser/sync/password_sync_bridge.h b/components/password_manager/core/browser/sync/password_sync_bridge.h
index b549e95..6608cba 100644
--- a/components/password_manager/core/browser/sync/password_sync_bridge.h
+++ b/components/password_manager/core/browser/sync/password_sync_bridge.h
@@ -63,6 +63,8 @@
bool SupportsGetStorageKey() const override;
void ApplyStopSyncChanges(std::unique_ptr<syncer::MetadataChangeList>
delete_metadata_change_list) override;
+ sync_pb::EntitySpecifics TrimRemoteSpecificsForCaching(
+ const sync_pb::EntitySpecifics& entity_specifics) override;
static std::string ComputeClientTagForTesting(
const sync_pb::PasswordSpecificsData& password_data);
diff --git a/components/password_manager/core/browser/sync/password_sync_bridge_unittest.cc b/components/password_manager/core/browser/sync/password_sync_bridge_unittest.cc
index 57c45d4..9e4fcba 100644
--- a/components/password_manager/core/browser/sync/password_sync_bridge_unittest.cc
+++ b/components/password_manager/core/browser/sync/password_sync_bridge_unittest.cc
@@ -1466,4 +1466,45 @@
EXPECT_FALSE(error);
}
+TEST_F(PasswordSyncBridgeTest,
+ TrimRemoteSpecificsForCachingPreservesOnlyUnknownFields) {
+ sync_pb::EntitySpecifics specifics_with_only_unknown_fields;
+ *specifics_with_only_unknown_fields.mutable_password()
+ ->mutable_client_only_encrypted_data()
+ ->mutable_unknown_fields() = "unknown_fields";
+
+ sync_pb::EntitySpecifics specifics;
+ sync_pb::PasswordSpecificsData* password_data =
+ specifics.mutable_password()->mutable_client_only_encrypted_data();
+ password_data->set_scheme(2);
+ password_data->set_signon_realm(kSignonRealm1);
+ password_data->set_origin("http://www.origin.com/");
+ password_data->set_action("action");
+ password_data->set_username_element("username_element");
+ password_data->set_username_value("username_value");
+ password_data->set_password_element("password_element");
+ password_data->set_password_value("password_value");
+ password_data->set_date_created(1000);
+ password_data->set_blacklisted(false);
+ password_data->set_type(0);
+ password_data->set_times_used(1);
+ password_data->set_display_name("display_name");
+ password_data->set_avatar_url("avatar_url");
+ password_data->set_federation_url("federation_url");
+ password_data->set_date_last_used(1000);
+ *password_data->mutable_password_issues() =
+ CreateSpecificsIssues({InsecureType::kLeaked});
+ password_data->set_date_password_modified_windows_epoch_micros(1000);
+
+ *specifics.mutable_password()
+ ->mutable_client_only_encrypted_data()
+ ->mutable_unknown_fields() = "unknown_fields";
+
+ sync_pb::EntitySpecifics trimmed_specifics =
+ bridge()->TrimRemoteSpecificsForCaching(specifics);
+
+ EXPECT_EQ(trimmed_specifics.SerializeAsString(),
+ specifics_with_only_unknown_fields.SerializeAsString());
+}
+
} // namespace password_manager
diff --git a/components/sync/model/model_type_sync_bridge.cc b/components/sync/model/model_type_sync_bridge.cc
index b6d63fac..626d46d2 100644
--- a/components/sync/model/model_type_sync_bridge.cc
+++ b/components/sync/model/model_type_sync_bridge.cc
@@ -68,6 +68,13 @@
return 0U;
}
+sync_pb::EntitySpecifics ModelTypeSyncBridge::TrimRemoteSpecificsForCaching(
+ const sync_pb::EntitySpecifics& entity_specifics) {
+ // Clears all fields by default to avoid the memory and I/O overhead of an
+ // additional copy of the data.
+ return sync_pb::EntitySpecifics();
+}
+
ModelTypeChangeProcessor* ModelTypeSyncBridge::change_processor() {
return change_processor_.get();
}
diff --git a/components/sync/model/model_type_sync_bridge.h b/components/sync/model/model_type_sync_bridge.h
index b693aaa..734de00 100644
--- a/components/sync/model/model_type_sync_bridge.h
+++ b/components/sync/model/model_type_sync_bridge.h
@@ -198,6 +198,17 @@
// SyncableService by other means.
virtual size_t EstimateSyncOverheadMemoryUsage() const;
+ // Returns a copy of |entity_specifics| where fields that do not need to be
+ // preserved in EntityMetadata cache are cleared. This allows each data type
+ // to specify which fields are supported in the current version. This usually
+ // means all known proto fields (i.e. all except unknown proto fields synced
+ // from more recent versions of the browser) but not always, since there are
+ // cases where a proto field is defined, but its implementation is not
+ // complete yet or exists behind a feature flag.
+ // By default, empty EntitySpecifics is returned.
+ virtual sync_pb::EntitySpecifics TrimRemoteSpecificsForCaching(
+ const sync_pb::EntitySpecifics& entity_specifics);
+
// Needs to be informed about any model change occurring via Delete() and
// Put(). The changing metadata should be stored to persistent storage
// before or atomically with the model changes.
