Support to remove passwords by date_synced timestamp.

chrome/browser/password_manager/native_backend_gnome_x.h

 // Copyright (c) 2012 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 CHROME_BROWSER_PASSWORD_MANAGER_NATIVE_BACKEND_GNOME_X_H_
 #define CHROME_BROWSER_PASSWORD_MANAGER_NATIVE_BACKEND_GNOME_X_H_
 
 #include <gnome-keyring.h>
 
 #include <string>
@@ skipping 69 matching lines @@
 
   virtual bool Init() OVERRIDE;
 
   // Implements NativeBackend interface.
   virtual password_manager::PasswordStoreChangeList AddLogin(
       const autofill::PasswordForm& form) OVERRIDE;
   virtual bool UpdateLogin(
       const autofill::PasswordForm& form,
       password_manager::PasswordStoreChangeList* changes) OVERRIDE;
   virtual bool RemoveLogin(const autofill::PasswordForm& form) OVERRIDE;
-  virtual bool RemoveLoginsCreatedBetween(
-      const base::Time& delete_begin, const base::Time& delete_end) OVERRIDE;
+  virtual bool RemoveLoginsCreatedBetween(base::Time delete_begin,
+                                          base::Time delete_end) OVERRIDE;

[vabr (Chromium), 2014/06/17 15:28:40]

Please also comment on the special meaning of the default base::Time value for
|delete_end| (in the public methods at least, but ideally also in the private
ones as well).

[vabr (Chromium), 2014/06/17 16:53:30]

The appropriate place for the comment is actually in PasswordStoreX.

[vasilii, 2014/06/17 17:24:14]

Done.

+  virtual bool RemoveLoginsSyncedBetween(
+      base::Time delete_begin,
+      base::Time delete_end,
+      password_manager::PasswordStoreChangeList* changes) OVERRIDE;
   virtual bool GetLogins(const autofill::PasswordForm& form,
                          PasswordFormList* forms) OVERRIDE;
-  virtual bool GetLoginsCreatedBetween(const base::Time& get_begin,
-                                       const base::Time& get_end,
+  virtual bool GetLoginsCreatedBetween(base::Time get_begin, base::Time get_end,

[vabr (Chromium), 2014/06/17 15:28:40]

optional nit: Is this formatting according to clang-format?
It's probably fine with the style guide, I just guess clang-format would put
every argument on one line. I see a value in following clang-format in that
potential refactoring later can rely on running clang-format and inducing
changes only on places which were refactored.

As this repeats on couple of places, my suggestion would be to run git cl format
on the whole CL, and accept those changes unless there is a strong reason
against.

[vasilii, 2014/06/17 17:24:14]

Done.

                                        PasswordFormList* forms) OVERRIDE;
   virtual bool GetAutofillableLogins(PasswordFormList* forms) OVERRIDE;
   virtual bool GetBlacklistLogins(PasswordFormList* forms) OVERRIDE;
 
  private:
   // Adds a login form without checking for one to replace first.
   bool RawAddLogin(const autofill::PasswordForm& form);
 
   // Reads PasswordForms from the keyring with the given autofillability state.
   bool GetLoginsList(PasswordFormList* forms, bool autofillable);
 
   // Helper for GetLoginsCreatedBetween().
   bool GetAllLogins(PasswordFormList* forms);
 
+  // Retrieves password created/synced in the time interval.

[vabr (Chromium), 2014/06/17 15:28:40]

Please comment on the return value (also below).

[vasilii, 2014/06/17 17:24:14]

Done.

+  bool GetLoginsBetween(base::Time get_begin, base::Time get_end,
+                        bool date_is_creation,

[vabr (Chromium), 2014/06/17 15:28:40]

|date_is_creation| would deserve an explanation. I did not understand what that
means until I read the definition of RemoveLogins*Between in the .cc file.

Even better, I'd suggest turning it from a bool to an enum like
enum TimestampToCompare { CREATION_TIMESTAMP, SYNC_TIMESTAMP };

That would avoid the need to add the comment, and as a bonus, make the meaning
of the argument clear on the callsites.

[vasilii, 2014/06/17 17:24:14]

Done.

+                        PasswordFormList* forms);
+
+  // Removes password created/synced in the time interval.
+  bool RemoveLoginsBetween(base::Time get_begin, base::Time get_end,
+                           bool date_is_creation,
+                           password_manager::PasswordStoreChangeList* changes);
+
   // Generates a profile-specific app string based on profile_id_.
   std::string GetProfileSpecificAppString() const;
 
   // The local profile id, used to generate the app string.
   const LocalProfileId profile_id_;
 
   // The app string, possibly based on the local profile id.
   std::string app_string_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeBackendGnome);
 };
 
 #endif  // CHROME_BROWSER_PASSWORD_MANAGER_NATIVE_BACKEND_GNOME_X_H_