Support to remove passwords by date_synced timestamp.

chrome/browser/password_manager/native_backend_gnome_x.h

Index: chrome/browser/password_manager/native_backend_gnome_x.h
diff --git a/chrome/browser/password_manager/native_backend_gnome_x.h b/chrome/browser/password_manager/native_backend_gnome_x.h
index c5d2c2ecd40e5ed1acf8fc7063d1c45ff340d28a..3bedf0c385e8e45fa4c75948a72f9264460c5bd1 100644
--- a/chrome/browser/password_manager/native_backend_gnome_x.h
+++ b/chrome/browser/password_manager/native_backend_gnome_x.h
@@ -87,12 +87,15 @@ class NativeBackendGnome : public PasswordStoreX::NativeBackend,
       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;
@@ -107,6 +110,16 @@ class NativeBackendGnome : public PasswordStoreX::NativeBackend,
   // 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;