Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1314)

Unified Diff: chrome/browser/sync/glue/extension_model_associator.cc

Issue 2884027: Added checks to handle unsyncable extensions. (Closed)
Patch Set: Fixed comment typo Created 10 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/sync/glue/extension_model_associator.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/sync/glue/extension_model_associator.cc
diff --git a/chrome/browser/sync/glue/extension_model_associator.cc b/chrome/browser/sync/glue/extension_model_associator.cc
index 3ff404d57e8e89b26933d630af999736bb4c493e..c88ddbac51ca2dfea977090f43be048512938b9a 100644
--- a/chrome/browser/sync/glue/extension_model_associator.cc
+++ b/chrome/browser/sync/glue/extension_model_associator.cc
@@ -55,6 +55,7 @@ ExtensionData* SetOrCreateData(
void GetSyncableExtensionsClientData(
const ExtensionList& extensions,
ExtensionsService* extensions_service,
+ std::set<std::string>* unsyncable_extensions,
ExtensionDataMap* extension_data_map) {
DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
for (ExtensionList::const_iterator it = extensions.begin();
@@ -73,6 +74,8 @@ void GetSyncableExtensionsClientData(
// Assumes this is called before any server data is read.
DCHECK(extension_data.NeedsUpdate(ExtensionData::SERVER));
DCHECK(!extension_data.NeedsUpdate(ExtensionData::CLIENT));
+ } else {
+ unsyncable_extensions->insert(extension.id());
}
}
}
@@ -99,6 +102,7 @@ bool ExtensionModelAssociator::AssociateModels() {
return false;
}
+ std::set<std::string> unsyncable_extensions;
ExtensionDataMap extension_data_map;
// Read client-side data. Do this first so server data takes
@@ -109,13 +113,15 @@ bool ExtensionModelAssociator::AssociateModels() {
const ExtensionList* extensions = extensions_service->extensions();
CHECK(extensions);
GetSyncableExtensionsClientData(
- *extensions, extensions_service, &extension_data_map);
+ *extensions, extensions_service,
+ &unsyncable_extensions, &extension_data_map);
const ExtensionList* disabled_extensions =
extensions_service->disabled_extensions();
CHECK(disabled_extensions);
GetSyncableExtensionsClientData(
- *disabled_extensions, extensions_service, &extension_data_map);
+ *disabled_extensions, extensions_service,
+ &unsyncable_extensions, &extension_data_map);
}
// Read server-side data.
@@ -133,12 +139,19 @@ bool ExtensionModelAssociator::AssociateModels() {
LOG(ERROR) << "Invalid extensions specifics for id " << id;
return false;
}
- // Pass in false for merge_user_properties so client user
- // settings always take precedence.
- const ExtensionData& extension_data =
- *SetOrCreateData(&extension_data_map,
- ExtensionData::SERVER, false, server_data);
- DcheckIsExtensionSpecificsValid(extension_data.merged_data());
+ // Don't process server data for extensions we know are
+ // unsyncable. This doesn't catch everything, as if we don't
+ // have the extension already installed we can't check, but we
+ // also check at extension install time.
+ if (unsyncable_extensions.find(server_data.id()) ==
+ unsyncable_extensions.end()) {
+ // Pass in false for merge_user_properties so client user
+ // settings always take precedence.
+ const ExtensionData& extension_data =
+ *SetOrCreateData(&extension_data_map,
+ ExtensionData::SERVER, false, server_data);
+ DcheckIsExtensionSpecificsValid(extension_data.merged_data());
+ }
id = sync_node.GetSuccessorId();
}
}
@@ -204,9 +217,17 @@ bool ExtensionModelAssociator::OnClientUpdate(const std::string& id) {
LOG(ERROR) << kNoExtensionsFolderError;
return false;
}
-
- sync_pb::ExtensionSpecifics client_data;
- if (GetExtensionDataFromClient(id, &client_data)) {
+ ExtensionsService* extensions_service = GetExtensionsService();
+ Extension* extension = extensions_service->GetExtensionById(id, true);
+ if (extension) {
+ if (!IsExtensionSyncable(*extension)) {
+ LOG(DFATAL) << "OnClientUpdate() called for non-syncable extension "
+ << id;
+ return false;
+ }
+ sync_pb::ExtensionSpecifics client_data;
+ GetExtensionSpecifics(*extension, extensions_service, &client_data);
+ DcheckIsExtensionSpecificsValid(client_data);
ExtensionData extension_data =
ExtensionData::FromData(ExtensionData::CLIENT, client_data);
sync_pb::ExtensionSpecifics server_data;
@@ -241,13 +262,21 @@ bool ExtensionModelAssociator::OnClientUpdate(const std::string& id) {
}
void ExtensionModelAssociator::OnServerUpdate(
- const sync_pb::ExtensionSpecifics& server_data) {
+ const sync_pb::ExtensionSpecifics& server_data,
+ Extension* extension) {
DcheckIsExtensionSpecificsValid(server_data);
ExtensionData extension_data =
ExtensionData::FromData(ExtensionData::SERVER, server_data);
- sync_pb::ExtensionSpecifics client_data;
- if (GetExtensionDataFromClient(server_data.id(), &client_data)) {
- ExtensionData extension_data =
+ if (extension) {
+ if (!IsExtensionSyncable(*extension)) {
+ LOG(DFATAL) << "OnServerUpdate() called for non-syncable extension "
+ << extension->id();
+ return;
+ }
+ sync_pb::ExtensionSpecifics client_data;
+ GetExtensionSpecifics(*extension, GetExtensionsService(), &client_data);
+ DcheckIsExtensionSpecificsValid(client_data);
+ extension_data =
ExtensionData::FromData(ExtensionData::CLIENT, client_data);
extension_data.SetData(ExtensionData::SERVER, true, server_data);
}
@@ -261,16 +290,6 @@ void ExtensionModelAssociator::OnServerUpdate(
DCHECK(!extension_data.NeedsUpdate(ExtensionData::SERVER));
}
-void ExtensionModelAssociator::OnServerRemove(const std::string& id) {
- ExtensionsService* extensions_service = GetExtensionsService();
- Extension* extension = extensions_service->GetExtensionById(id, true);
- if (extension) {
- extensions_service->UninstallExtension(id, false);
- } else {
- LOG(ERROR) << "Trying to uninstall nonexistent extension " << id;
- }
-}
-
ExtensionsService* ExtensionModelAssociator::GetExtensionsService() {
CHECK(sync_service_);
Profile* profile = sync_service_->profile();
@@ -280,18 +299,6 @@ ExtensionsService* ExtensionModelAssociator::GetExtensionsService() {
return extensions_service;
}
-bool ExtensionModelAssociator::GetExtensionDataFromClient(
- const std::string& id, sync_pb::ExtensionSpecifics* client_data) {
- ExtensionsService* extensions_service = GetExtensionsService();
- Extension* extension = extensions_service->GetExtensionById(id, true);
- if (!extension) {
- return false;
- }
- GetExtensionSpecifics(*extension, extensions_service, client_data);
- DcheckIsExtensionSpecificsValid(*client_data);
- return true;
-}
-
bool ExtensionModelAssociator::GetExtensionDataFromServer(
const std::string& id, sync_api::WriteTransaction* trans,
const sync_api::ReadNode& root,
@@ -360,6 +367,11 @@ void ExtensionModelAssociator::TryUpdateClient(
const std::string& id = specifics.id();
Extension* extension = extensions_service->GetExtensionById(id, true);
if (extension) {
+ if (!IsExtensionSyncable(*extension)) {
+ LOG(DFATAL) << "TryUpdateClient() called for non-syncable extension "
+ << extension->id();
+ return;
+ }
SetExtensionProperties(specifics, extensions_service, extension);
{
sync_pb::ExtensionSpecifics extension_specifics;
« no previous file with comments | « chrome/browser/sync/glue/extension_model_associator.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698