[Sync] Implementation of model association for passwords using sync API

chrome/browser/password_manager/password_syncable_service.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/password_manager/password_syncable_service.h"
 
 #include "base/location.h"
+#include "base/memory/scoped_vector.h"
+#include "base/metrics/histogram.h"
+#include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/password_manager/password_store.h"
 #include "components/autofill/core/common/password_form.h"
 #include "net/base/escape.h"
 #include "sync/api/sync_error_factory.h"
 
 namespace {
 
-// Converts the |PasswordSpecifics| obtained from sync to an
-// object of type |PasswordForm|.
-void ExtractPasswordFromSpecifics(
-     const sync_pb::PasswordSpecificsData& password,
-     autofill::PasswordForm* new_password) {
-  new_password->scheme =
-      static_cast<autofill::PasswordForm::Scheme>(password.scheme());
-  new_password->signon_realm = password.signon_realm();
-  new_password->origin = GURL(password.origin());
-  new_password->action = GURL(password.action());
-  new_password->username_element =
-      UTF8ToUTF16(password.username_element());
-  new_password->password_element =
-      UTF8ToUTF16(password.password_element());
-  new_password->username_value =
-      UTF8ToUTF16(password.username_value());
-  new_password->password_value =
-      UTF8ToUTF16(password.password_value());
-  new_password->ssl_valid = password.ssl_valid();
-  new_password->preferred = password.preferred();
-  new_password->date_created =
-      base::Time::FromInternalValue(password.date_created());
-  new_password->blacklisted_by_user =
-      password.blacklisted();
-}
-
 // Merges the sync password (obtained from the password specifics) and
 // local password and stores the output in the |new_password_form| pointer.
 bool MergeLocalAndSyncPasswords(
     const sync_pb::PasswordSpecificsData& password_specifics,
     const autofill::PasswordForm& password_form,
     autofill::PasswordForm* new_password_form) {
   if (password_specifics.scheme() == password_form.scheme &&
       password_form.signon_realm == password_specifics.signon_realm() &&
       password_form.origin.spec() == password_specifics.origin() &&
       password_form.action.spec() == password_specifics.action() &&
@@ skipping 12 matching lines @@
       password_specifics.blacklisted() ==
           password_form.blacklisted_by_user) {
     return false;
   }
 
   // If the passwords differ, take the one that was created more recently.
   if (base::Time::FromInternalValue(password_specifics.date_created()) <=
           password_form.date_created) {
     *new_password_form = password_form;
   } else {
-    ExtractPasswordFromSpecifics(password_specifics, new_password_form);
+    PasswordSyncableService::ExtractPasswordFromSpecifics(
+        password_specifics,
+        new_password_form);
   }
 
   return true;
 }
 
 }  // namespace
 
 PasswordSyncableService::PasswordSyncableService(
     scoped_refptr<PasswordStore> password_store)
     : password_store_(password_store) {
 }
 
 PasswordSyncableService::~PasswordSyncableService() {}
 
+void PasswordSyncableService::CreateOrUpdateEntry(
+    const syncer::SyncData& data,
+    PasswordEntryMap* loaded_data,
+    ScopedVector<autofill::PasswordForm>* new_entries,
+    ScopedVector<autofill::PasswordForm>* updated_entries,
+    syncer::SyncChangeList* updated_db_entries) {
+  const sync_pb::EntitySpecifics& specifics = data.GetSpecifics();
+  const sync_pb::PasswordSpecificsData& password_specifics(
+      specifics.password().client_only_encrypted_data());
+
+  std::string tag = MakeTag(password_specifics);
+
+  // Find if the data from sync is already in password store.
+  PasswordEntryMap::iterator it = loaded_data->find(tag);
+
+  if (it == loaded_data->end()) {
+    // The sync data is not in password store, so we need to create it in
+    // password store. Add the entry to the new_entries list.
+    autofill::PasswordForm* new_password = new autofill::PasswordForm();
+    ExtractPasswordFromSpecifics(password_specifics, new_password);
+    // The new_password pointer is owned by the caller.
+    new_entries->push_back(new_password);
+  } else {
+    // The entry is in password store. If the entries are not identical the
+    // entries need to be merged.
+    scoped_ptr<autofill::PasswordForm> new_password(
+        new autofill::PasswordForm());
+    if (MergeLocalAndSyncPasswords(password_specifics,
+                                   **(it->second),
+                                   new_password.get())) {
+      updated_db_entries->push_back(
+          syncer::SyncChange(FROM_HERE,
+                             syncer::SyncChange::ACTION_UPDATE,
+                             CreateSyncData(*(new_password.get()))));
+
+      updated_entries->push_back(new_password.release());
+    }
+    // Remove the entry from the entry map to indicate a match has been found.
+    // Entries that remain in the map at the end of associating all sync entries
+    // will be treated as additions that needs to be propagated to sync.
+    loaded_data->erase(it);
+  }
+}
+
 syncer::SyncMergeResult
 PasswordSyncableService::MergeDataAndStartSyncing(
     syncer::ModelType type,
     const syncer::SyncDataList& initial_sync_data,
     scoped_ptr<syncer::SyncChangeProcessor> sync_processor,
     scoped_ptr<syncer::SyncErrorFactory> sync_error_factory) {
   syncer::SyncMergeResult merge_result(type);
   sync_error_factory_ = sync_error_factory.Pass();
   sync_processor_ = sync_processor.Pass();
 
-  merge_result.set_error(sync_error_factory->CreateAndUploadError(
-      FROM_HERE,
-      "Password Syncable Service Not Implemented."));
+  ScopedVector<autofill::PasswordForm> password_entries;
+  if (!password_store_->FillAutofillableLogins(&(password_entries.get())) ||
+      !password_store_->FillBlacklistLogins(&(password_entries.get()))) {
+    // Password store often fails to load passwords. Track failures with UMA.
+    // (http://crbug.com/249000)
+    UMA_HISTOGRAM_ENUMERATION("Sync.LocalDataFailedToLoad",
+                              ModelTypeToHistogramInt(syncer::PASSWORDS),
+                              syncer::MODEL_TYPE_COUNT);
+    merge_result.set_error(sync_error_factory->CreateAndUploadError(
+        FROM_HERE,
+        "Failed to get passwords from store."));
+    return merge_result;;
+  }
+
+  PasswordEntryMap new_db_entries;
+  for (std::vector<autofill::PasswordForm*>::iterator
+           it = password_entries.begin();
+       it != password_entries.end();
+       ++it) {
+     // We add all the db entries as |new_db_entries| initially. While
+     // association entries with matching sync entries will be removed and this
+     // list will only contain entries that are not in sync.
+     new_db_entries[MakeTag(**it)] = it;
+  }
+
+  // List that contains the entries that are known only to sync.
+  ScopedVector<autofill::PasswordForm> new_synced_entries;

[Nicolas Zea, 2013/10/25 20:20:13]

nit: I think the name here would better represent whether or not the data is new
to the db. In other words, because the data is known only to sync, it should be
added to a list of "new_db_entries". Similarly, the variable below would be
"updated_db_entries".

[lipalani1, 2013/10/29 20:16:36]

May be I am missing something here:
The names refer to the state of the item in the respective stores(sync or
local). new_sync_entries are items that are new in sync sync db. 

there is already a variable above called new_db_entries(which are items new in
passwords db!)

I suppose you mean to make them refer to what action we will do on those
items.(i.e., switch these names) I am OK either way, let me know.

My assumption is with the comment above each variable it should be easy to
follow.
On 2013/10/25 20:20:13, Nicolas Zea wrote:

+
+  // List that contains the entries that are known to both sync and db but
+  // have to be updated in sync.
+  ScopedVector<autofill::PasswordForm> updated_sync_entries;
+
+  // Changes from password db that needs to be propagated to sync.
+  syncer::SyncChangeList db_changes;
+  for (syncer::SyncDataList::const_iterator sync_iter =
+           initial_sync_data.begin();
+       sync_iter != initial_sync_data.end(); ++sync_iter) {
+    CreateOrUpdateEntry(*sync_iter,
+                        &new_db_entries,
+                        &new_synced_entries,
+                        &updated_sync_entries,
+                        &db_changes);
+  }
+
+  WriteToPasswordStore(new_synced_entries.get(),
+                       updated_sync_entries.get());
+
+  for (PasswordEntryMap::iterator it = new_db_entries.begin();
+       it != new_db_entries.end();
+       ++it) {
+    db_changes.push_back(syncer::SyncChange(FROM_HERE,
+                              syncer::SyncChange::ACTION_ADD,
+                              CreateSyncData(**(it->second))));
+  }
+
+  merge_result.set_error(

[Nicolas Zea, 2013/10/25 20:20:13]

could you also fill calculate the other variables in merge result? (items
before, after, added, etc.)

[lipalani1, 2013/10/29 20:16:36]

Done.

+      sync_processor_->ProcessSyncChanges(FROM_HERE, db_changes));
   return merge_result;
 }
 
 void PasswordSyncableService::StopSyncing(syncer::ModelType type) {
 }
 
 syncer::SyncDataList PasswordSyncableService::GetAllSyncData(
     syncer::ModelType type) const {
   syncer::SyncDataList sync_data;
   return sync_data;
 }
 
 syncer::SyncError PasswordSyncableService::ProcessSyncChanges(
     const tracked_objects::Location& from_here,
     const syncer::SyncChangeList& change_list) {
   syncer::SyncError error(FROM_HERE,
                           syncer::SyncError::UNRECOVERABLE_ERROR,
                           "Password Syncable Service Not Implemented.",
                           syncer::PASSWORDS);
   return error;
 }
 
 void PasswordSyncableService::WriteToPasswordStore(
-    PasswordForms* new_entries,
-    PasswordForms* updated_entries) {
+    const PasswordForms& new_entries,
+    const PasswordForms& updated_entries) {
   for (std::vector<autofill::PasswordForm*>::const_iterator it =
-           new_entries->begin();
-       it != new_entries->end();
+           new_entries.begin();
+       it != new_entries.end();
        ++it) {
     password_store_->AddLoginImpl(**it);
   }
 
   for (std::vector<autofill::PasswordForm*>::const_iterator it =
-           updated_entries->begin();
-       it != updated_entries->end();
+           updated_entries.begin();
+       it != updated_entries.end();
        ++it) {
     password_store_->UpdateLoginImpl(**it);
   }
 
-  if (!new_entries->empty() || !updated_entries->empty()) {
+  if (!new_entries.empty() || !updated_entries.empty()) {
     // We have to notify password store observers of the change by hand since
     // we use internal password store interfaces to make changes synchronously.
-    password_store_->PostNotifyLoginsChanged();
+    NotifyPasswordStore();
   }
 }
 
+void PasswordSyncableService::NotifyPasswordStore() {
+  password_store_->PostNotifyLoginsChanged();
+}
+
 syncer::SyncData PasswordSyncableService::CreateSyncData(
     const autofill::PasswordForm& password_form) {
   sync_pb::EntitySpecifics password_data;
   sync_pb::PasswordSpecificsData* password_specifics =
       password_data.mutable_password()->mutable_client_only_encrypted_data();
   password_specifics->set_scheme(password_form.scheme);
   password_specifics->set_signon_realm(password_form.signon_realm);
   password_specifics->set_origin(password_form.origin.spec());
   password_specifics->set_action(password_form.action.spec());
   password_specifics->set_username_element(
@@ skipping 41 matching lines @@
 // static
 std::string PasswordSyncableService::MakeTag(
     const sync_pb::PasswordSpecificsData& password) {
   return MakeTag(password.origin(),
                  password.username_element(),
                  password.username_value(),
                  password.password_element(),
                  password.signon_realm());
 }
 
+// static
+void PasswordSyncableService::ExtractPasswordFromSpecifics(
+     const sync_pb::PasswordSpecificsData& password,
+     autofill::PasswordForm* new_password) {
+  new_password->scheme =
+      static_cast<autofill::PasswordForm::Scheme>(password.scheme());
+  new_password->signon_realm = password.signon_realm();
+  new_password->origin = GURL(password.origin());
+  new_password->action = GURL(password.action());
+  new_password->username_element =
+      UTF8ToUTF16(password.username_element());
+  new_password->password_element =
+      UTF8ToUTF16(password.password_element());
+  new_password->username_value =
+      UTF8ToUTF16(password.username_value());
+  new_password->password_value =
+      UTF8ToUTF16(password.password_value());
+  new_password->ssl_valid = password.ssl_valid();
+  new_password->preferred = password.preferred();
+  new_password->date_created =
+      base::Time::FromInternalValue(password.date_created());
+  new_password->blacklisted_by_user =
+      password.blacklisted();
+}
+