Clipboard Android - Store and Read Last Modified Time from Prefs

ui/base/clipboard/clipboard_android.cc

Index: ui/base/clipboard/clipboard_android.cc
diff --git a/ui/base/clipboard/clipboard_android.cc b/ui/base/clipboard/clipboard_android.cc
index b33b78fcd7fcadcd517ef93cab653c0056408ee9..3661cf16a3ac0063c71f070c23b163984025c4a6 100644
--- a/ui/base/clipboard/clipboard_android.cc
+++ b/ui/base/clipboard/clipboard_android.cc
@@ -14,6 +14,8 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock.h"
 #include "base/time/time.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
 #include "jni/Clipboard_jni.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/geometry/size.h"
@@ -40,6 +42,10 @@ namespace ui {
 
 namespace {
 
+// Stores the last modified time in base::Time's internal format (int64) as
+// a pref in local store.  (I.e., this is not a per-profile pref.)
+const char kClipboardLastModifiedTimePref[] = "ui.clipboard.last_modified_time";

[Bernhard Bauer, 2017/04/26 10:03:42]

Oh, usually pref names are defined in chrome/common/pref_names.{h,cc}. Sorry, I
totally missed that earlier (and this class couldn't have accessed
chrome/common/ anyway, but now it should work).

[Mark P, 2017/04/26 22:18:32]

Defined them there now.

For what it's worth, I originally decided to put them in this file for a
reasonable reason.  I looked around for pref names and found many files where
prefs are defined locally in a file, always in cases when the pref is not
accessed elsewhere.  I inferred the putting things in common was only
necessary/required when a pref name needed a common place to live (i.e., be
accessed from multiple files across the codebase).

[Bernhard Bauer, 2017/04/27 09:27:17]

That probably was more of an oversight on part of the reviewer ;-) I think the
idea is to have a central place for all prefs (well, at least in a given layer).
But anyway, thanks for the fix.

[Mark P, 2017/04/27 19:25:59]

Acknowledged.

+
 // Various formats we support.
 const char kURLFormat[] = "url";
 const char kPlainTextFormat[] = "text";
@@ -52,6 +58,7 @@ const char kBookmarkFormat[] = "bookmark";
 class ClipboardMap {
  public:
   ClipboardMap();
+  void SetLocalState(PrefService* local_state);
   std::string Get(const std::string& format);
   uint64_t GetSequenceNumber() const;
   base::Time GetLastModifiedTime() const;
@@ -69,7 +76,12 @@ class ClipboardMap {
     kPreparingCommit,
   };
 
+  // Updates |last_modified_time_| to |time| and writes it to |local_state_|.
+  void UpdateLastModifiedTime(const base::Time time);

[Bernhard Bauer, 2017/04/26 10:03:42]

Small nit: The const now only makes the local variable const, which we usually
don't bother with.

[Mark P, 2017/04/26 22:18:32]

Done.

+
+  // Updates |map_| and |map_state_| if necessary by fetching data from Java.
   void UpdateFromAndroidClipboard();
+
   std::map<std::string, std::string> map_;
   MapState map_state_;
   base::Lock lock_;
@@ -77,6 +89,8 @@ class ClipboardMap {
   uint64_t sequence_number_;
   base::Time last_modified_time_;
 
+  PrefService* local_state_;
+
   // Java class and methods for the Android ClipboardManager.
   ScopedJavaGlobalRef<jobject> clipboard_manager_;
 };
@@ -87,6 +101,12 @@ ClipboardMap::ClipboardMap() : map_state_(MapState::kOutOfDate) {
   DCHECK(clipboard_manager_.obj());
 }
 
+void ClipboardMap::SetLocalState(PrefService* local_state) {
+  local_state_ = local_state;
+  last_modified_time_ = base::Time::FromInternalValue(
+      local_state_->GetInt64(kClipboardLastModifiedTimePref));
+}
+
 std::string ClipboardMap::Get(const std::string& format) {
   base::AutoLock lock(lock_);
   UpdateFromAndroidClipboard();
@@ -103,7 +123,7 @@ base::Time ClipboardMap::GetLastModifiedTime() const {
 }
 
 void ClipboardMap::ClearLastModifiedTime() {
-  last_modified_time_ = base::Time();
+  UpdateLastModifiedTime(base::Time());
 }
 
 bool ClipboardMap::HasFormat(const std::string& format) {
@@ -114,7 +134,7 @@ bool ClipboardMap::HasFormat(const std::string& format) {
 
 void ClipboardMap::OnPrimaryClipboardChanged() {
   sequence_number_++;
-  last_modified_time_ = base::Time::Now();
+  UpdateLastModifiedTime(base::Time::Now());
   map_state_ = MapState::kOutOfDate;
 }
 
@@ -151,7 +171,7 @@ void ClipboardMap::CommitToAndroidClipboard() {
   }
   map_state_ = MapState::kUpToDate;
   sequence_number_++;
-  last_modified_time_ = base::Time::Now();
+  UpdateLastModifiedTime(base::Time::Now());
 }
 
 void ClipboardMap::Clear() {
@@ -161,7 +181,7 @@ void ClipboardMap::Clear() {
   Java_Clipboard_clear(env, clipboard_manager_);
   map_state_ = MapState::kUpToDate;
   sequence_number_++;
-  last_modified_time_ = base::Time::Now();
+  UpdateLastModifiedTime(base::Time::Now());
 }
 
 // Add a key:jstr pair to map, but only if jstr is not null, and also
@@ -177,6 +197,15 @@ void AddMapEntry(JNIEnv* env,
   }
 }
 
+void ClipboardMap::UpdateLastModifiedTime(const base::Time time) {
+  last_modified_time_ = time;
+  // |local_state_| may be null in tests.
+  if (local_state_) {
+    local_state_->SetInt64(kClipboardLastModifiedTimePref,
+                           last_modified_time_.ToInternalValue());
+  }
+}
+
 void ClipboardMap::UpdateFromAndroidClipboard() {
   DCHECK_NE(MapState::kPreparingCommit, map_state_);
   if (map_state_ == MapState::kUpToDate)
@@ -297,6 +326,15 @@ Clipboard* Clipboard::Create() {
 
 // ClipboardAndroid implementation.
 
+// static
+void ClipboardAndroid::RegisterPrefs(PrefRegistrySimple* registry) {
+  registry->RegisterInt64Pref(kClipboardLastModifiedTimePref, 0u);
+}
+
+void ClipboardAndroid::SetLocalState(PrefService* local_state) {
+  g_map.Get().SetLocalState(local_state);
+}
+
 void ClipboardAndroid::OnPrimaryClipChanged(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& obj) {