Make Android Clipboard Keep Track of Last Modified Time

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 46b536e649325d72d74681619a087f7eec9fd0a2..3e76e80658cf0c49973f1520bd336afc58aa086d 100644
--- a/ui/base/clipboard/clipboard_android.cc
+++ b/ui/base/clipboard/clipboard_android.cc
@@ -4,6 +4,9 @@
 
 #include "ui/base/clipboard/clipboard_android.h"
 
+#include <algorithm>
+
+#include "base/android/context_utils.h"
 #include "base/android/jni_string.h"
 #include "base/lazy_instance.h"
 #include "base/stl_util.h"
@@ -47,6 +50,7 @@ class ClipboardMap {
  public:
   ClipboardMap();
   std::string Get(const std::string& format);
+  int64_t GetLastClipboardChangeTimeInMillis();
   bool HasFormat(const std::string& format);
   void Set(const std::string& format, const std::string& data);
   void CommitToAndroidClipboard();
@@ -57,6 +61,8 @@ class ClipboardMap {
   std::map<std::string, std::string> map_;
   base::Lock lock_;
 
+  int64_t last_clipboard_change_time_ms_;
+
   // Java class and methods for the Android ClipboardManager.
   ScopedJavaGlobalRef<jobject> clipboard_manager_;
 };
@@ -74,6 +80,12 @@ std::string ClipboardMap::Get(const std::string& format) {
   return it == map_.end() ? std::string() : it->second;
 }
 
+int64_t ClipboardMap::GetLastClipboardChangeTimeInMillis() {
+  base::AutoLock lock(lock_);
+  UpdateFromAndroidClipboard();
+  return last_clipboard_change_time_ms_;
+}
+
 bool ClipboardMap::HasFormat(const std::string& format) {
   base::AutoLock lock(lock_);
   UpdateFromAndroidClipboard();
@@ -158,6 +170,9 @@ void ClipboardMap::UpdateFromAndroidClipboard() {
 
   AddMapEntry(env, &android_clipboard_state, kPlainTextFormat, jtext);
   AddMapEntry(env, &android_clipboard_state, kHTMLFormat, jhtml);
+  last_clipboard_change_time_ms_ =

[Ted C, 2017/03/30 16:55:21]

How often are we going to be asking for the clipboard age?  Every omnibox
keystroke?

I'm wondering if we can use the last_clipboard_change_time_ms_ to actually not
pull the text and html text if it hasn't changed (it might need to be a
base::Optional or something to handle the initial unset state).

JNI for strings isn't "super" expensive but it isn't free either and if someone
were to copy a very long string, this could actually get somewhat expensive.

Another option entirely would be to not hook the change time into the update
from android call and have the above getter just call directly into the java
side.  That would avoid any of this caching logic as well, which doesn't buy us
much here.

[Mark P, 2017/03/30 16:59:26]

Not per keystroke.  The current plan only involves checking for and showing the
clipboard suggestion on omnibox focus.  It disappears when the user types.

Given this answer, do you consider either of the suggestions below necessary?

[Ted C, 2017/03/30 17:02:06]

I would personally probably inline the JNI into the above getter, but I'll defer
to you.  This class is weird in general and I don't know why there is a map at
all.

[Mark P, 2017/03/30 17:33:36]

I don't understand the current structure either.  I'll leave this comment here
so dcheng@ can comment/decide.

[dcheng, 2017/03/30 19:01:11]

Either option seems reasonable here. The original idea was to allow
ClipboardAndroid to accumulate changes that couldn't easily be persisted back to
the Android clipboard, but make sure that the internal ClipboardAndroid state
and the Android system clipboard state remained roughly in sync.

That being said, I'd probably use an actual sequence number rather than relying
on the last changed time for shortcutting this work. It should be pretty easy to
implement now that we have a clipboard change listener, right?

(And this would let us implement ClipboardAndroid::GetSequenceNumber correctly
as well)

[Mark P, 2017/03/30 19:15:03]

Ah, that makes sense (as an ambition).
Actually, for the changelist that comes next, I need the actual timestamp. 
(This is to get the omnibox to suggest URLs in the clipboard, and we want it not
to suggest URLs that are too old.  Hence, the timestamp; a sequence number
doesn't suffice.)

+      Java_Clipboard_getClipboardContentChangeTimeInMillis(env,
+                                                           clipboard_manager_);
 
   if (!MapIsSubset(android_clipboard_state, map_))
     android_clipboard_state.swap(map_);
@@ -399,6 +414,12 @@ void ClipboardAndroid::ReadData(const Clipboard::FormatType& format,
   *result = g_map.Get().Get(format.ToString());
 }
 
+base::Time ClipboardAndroid::GetClipboardLastModifiedTime() const {
+  DCHECK(CalledOnValidThread());
+  return base::Time::FromJavaTime(
+      g_map.Get().GetLastClipboardChangeTimeInMillis());
+}
+
 // Main entry point used to write several values in the clipboard.
 void ClipboardAndroid::WriteObjects(ClipboardType type,
                                     const ObjectMap& objects) {