Password Login Database: report correct changes from AddLogin().

components/password_manager/core/browser/login_database.h

 // Copyright 2014 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.
 
 #ifndef COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LOGIN_DATABASE_H_
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LOGIN_DATABASE_H_
 
 #include <string>
 #include <vector>
 
 #include "base/files/file_path.h"
 #include "base/pickle.h"
 #include "base/strings/string16.h"
+#include "components/password_manager/core/browser/password_store_change.h"
 #include "components/password_manager/core/browser/psl_matching_helper.h"
 #include "sql/connection.h"
 #include "sql/meta_table.h"
 
-namespace autofill {
-struct PasswordForm;
-}  // namespace autofill
-
 namespace password_manager {
 
 // Interface to the database storage of login information, intended as a helper
 // for PasswordStore on platforms that need internal storage of some or all of
 // the login information.
 class LoginDatabase {
  public:
   LoginDatabase();
   virtual ~LoginDatabase();
 
   // Initialize the database with an sqlite file at the given path.
   // If false is returned, no other method should be called.
   bool Init(const base::FilePath& db_path);
 
   // Reports usage metrics to UMA.
   void ReportMetrics();
 
-  // Adds |form| to the list of remembered password forms.
-  bool AddLogin(const autofill::PasswordForm& form);
+  // Adds |form| to the list of remembered password forms. Returns the list of
+  // changes applied ({}, {ADD}, {DELETE, ADD}). In the latter case the previous

[Garrett Casto, 2014/05/14 07:46:35]

nit: It's REMOVE not DELETE.

[vasilii, 2014/05/14 11:20:18]

Done.

+  // password value isn't read. Thus only the UNIQUE key columns contain the

[Garrett Casto, 2014/05/14 07:46:35]

The last two sentences are pretty confusing. I think what your trying to say is
that if the function returns {REMOVE, ADD} then the returned
PasswordStoreChangeList will have the REMOVE associated with the form that was
added not the one that was removed. Would you mind trying to clarify the
language a little bit?

I don't think that this is a problem in practice, since sync only cares about
the key, though documenting this irregularity is good.

[vasilii, 2014/05/14 11:20:18]

Done.

+  // correct value.
+  PasswordStoreChangeList AddLogin(const autofill::PasswordForm& form);
 
   // Updates remembered password form. Returns true on success and sets
   // items_changed (if non-NULL) to the number of logins updated.
   bool UpdateLogin(const autofill::PasswordForm& form, int* items_changed);
 
   // Removes |form| from the list of remembered password forms.
   bool RemoveLogin(const autofill::PasswordForm& form);
 
   // Removes all logins created from |delete_begin| onwards (inclusive) and
   // before |delete_end|. You may use a null Time value to do an unbounded
@@ skipping 86 matching lines @@
   sql::MetaTable meta_table_;
 
   PSLMatchingHelper psl_helper_;
 
   DISALLOW_COPY_AND_ASSIGN(LoginDatabase);
 };
 
 }  // namespace password_manager
 
 #endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_LOGIN_DATABASE_H_