Enable pasting HTML content from the Android clipboard

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 34d4a83bdaaaac38400bb66ec504c345c8669788..46b65e840c12973c29ae44ecd5c9d345812cd91c 100644
--- a/ui/base/clipboard/clipboard_android.cc
+++ b/ui/base/clipboard/clipboard_android.cc
@@ -133,35 +133,31 @@ void ClipboardMap::SyncWithAndroidClipboard() {
   lock_.AssertAcquired();
   JNIEnv* env = AttachCurrentThread();
 
+  // Update the plain text clipboard entry
   std::map<std::string, std::string>::const_iterator it =
     map_.find(kPlainTextFormat);
-
-  if (!Java_Clipboard_hasPlainText(env, clipboard_manager_.obj())) {
-    if (it != map_.end())
-      // We have plain text on this side, but Android doesn't. Nuke ours.
-      map_.clear();
-    return;
-  }
-
-  ScopedJavaLocalRef<jstring> java_string =
+  ScopedJavaLocalRef<jstring> java_string_text =
       Java_Clipboard_getCoercedText(env, clipboard_manager_.obj());
-
-  if (!java_string.obj()) {
-    // Tolerate a null value from the Java side, even though that should not
-    // happen since hasPlainText has already returned true.
-    // Should only happen if someone is using the clipboard on multiple
-    // threads and clears it out after hasPlainText but before we get here...
-    if (it != map_.end())
+  if (java_string_text.obj()) {
+    std::string android_string = ConvertJavaStringToUTF8(java_string_text);
+    if (it == map_.end() || it->second != android_string) {

[joth, 2013/11/12 01:45:39]

the change from hasPlainText() method to the code you have here makes me
nervous, mostly because this has no automated tests and I don't know what the
manual test cases would be for this.

Suggestions welcome on how to ease my nerves.

[Kristian Monsen, 2013/11/12 04:27:23]

I mostly did not want to quit when there was not plain text in case there was
html text. That being said, hasPlainText needed to get the text and check if it
was null, so I don't think it optimized anything, but did it twice instead. The
if (java_string_text.obj()) should do the same. Also not that the hasPlainText
was potentially not safe in case someone messes with the clipboard in between.

There is not really that much code change here, this code is mostly the same as
the one git thinks was deleted just below. I just mixed it up a bit since we
don't want to return before adding the html text.

+      // There is a different string in the Android clipboard than we have.
+      // Clear the map on our side.
+      map_.clear();
+      map_[kPlainTextFormat] = android_string;
+    }
+  } else {
+    if (it != map_.end()) {
       // We have plain text on this side, but Android doesn't. Nuke ours.
       map_.clear();
-    return;
+    }
   }
 
-  // If Android text differs from ours (or we have none), then copy Android's.
-  std::string android_string = ConvertJavaStringToUTF8(java_string);
-  if (it == map_.end() || it->second != android_string) {
-    map_.clear();
-    map_[kPlainTextFormat] = android_string;
+  // Update the html clipboard entry
+  ScopedJavaLocalRef<jstring> java_string_html =
+      Java_Clipboard_getHtmlText(env, clipboard_manager_.obj());
+  if (java_string_html.obj()) {
+    map_[kHTMLFormat] = ConvertJavaStringToUTF8(java_string_html);

[joth, 2013/11/12 01:45:39]

the memory overhead of stashing two copies of everything in the clipboard in a
global map seems a bit messy (the long term memory cost).
If user copies a massive HTML doc from one app's webview into another, both
processes have 2 x massive doc in RAM until they each do another clipboard
operation.

probably the best answer is to wire this thing up to
base::MemoryPressureListener (e.g. to clear the map on the moderate level).
maybe a new crbug for that?

[Kristian Monsen, 2013/11/12 04:27:23]

We don't really need the map as I understand, can just query android when
needed. But that is not for this CL.

   }
 }
 
@@ -200,9 +196,9 @@ Clipboard::~Clipboard() {
 }
 
 // Main entry point used to write several values in the clipboard.
-void Clipboard::WriteObjects(ClipboardType type, const ObjectMap& objects) {
+void Clipboard::WriteObjects(Buffer buffer, const ObjectMap& objects) {
   DCHECK(CalledOnValidThread());
-  DCHECK_EQ(type, CLIPBOARD_TYPE_COPY_PASTE);
+  DCHECK_EQ(buffer, BUFFER_STANDARD);

[joth, 2013/11/12 01:45:39]

are all these edits from here down intended? look like M30 / master versions
skew mess.

[Kristian Monsen, 2013/11/12 02:17:36]

No, need a manual merge here. Will fix.

   g_map.Get().Clear();
   for (ObjectMap::const_iterator iter = objects.begin();
        iter != objects.end(); ++iter) {
@@ -210,7 +206,7 @@ void Clipboard::WriteObjects(ClipboardType type, const ObjectMap& objects) {
   }
 }
 
-uint64 Clipboard::GetSequenceNumber(ClipboardType /* type */) {
+uint64 Clipboard::GetSequenceNumber(Clipboard::Buffer /* buffer */) {
   DCHECK(CalledOnValidThread());
   // TODO: implement this. For now this interface will advertise
   // that the clipboard never changes. That's fine as long as we
@@ -219,23 +215,22 @@ uint64 Clipboard::GetSequenceNumber(ClipboardType /* type */) {
 }
 
 bool Clipboard::IsFormatAvailable(const Clipboard::FormatType& format,
-                                  ClipboardType type) const {
+                                  Clipboard::Buffer buffer) const {
   DCHECK(CalledOnValidThread());
-  DCHECK_EQ(type, CLIPBOARD_TYPE_COPY_PASTE);
+  DCHECK_EQ(buffer, BUFFER_STANDARD);
   return g_map.Get().HasFormat(format.data());
 }
 
-void Clipboard::Clear(ClipboardType type) {
+void Clipboard::Clear(Buffer buffer) {
   DCHECK(CalledOnValidThread());
-  DCHECK_EQ(type, CLIPBOARD_TYPE_COPY_PASTE);
+  DCHECK_EQ(buffer, BUFFER_STANDARD);
   g_map.Get().Clear();
 }
 
-void Clipboard::ReadAvailableTypes(ClipboardType type,
-                                   std::vector<string16>* types,
+void Clipboard::ReadAvailableTypes(Buffer buffer, std::vector<string16>* types,
                                    bool* contains_filenames) const {
   DCHECK(CalledOnValidThread());
-  DCHECK_EQ(type, CLIPBOARD_TYPE_COPY_PASTE);
+  DCHECK_EQ(buffer, BUFFER_STANDARD);
 
   if (!types || !contains_filenames) {
     NOTREACHED();
@@ -248,28 +243,29 @@ void Clipboard::ReadAvailableTypes(ClipboardType type,
   *contains_filenames = false;
 }
 
-void Clipboard::ReadText(ClipboardType type, string16* result) const {
+void Clipboard::ReadText(Clipboard::Buffer buffer, string16* result) const {
   DCHECK(CalledOnValidThread());
-  DCHECK_EQ(type, CLIPBOARD_TYPE_COPY_PASTE);
+  DCHECK_EQ(buffer, BUFFER_STANDARD);
   std::string utf8;
-  ReadAsciiText(type, &utf8);
+  ReadAsciiText(buffer, &utf8);
   *result = UTF8ToUTF16(utf8);
 }
 
-void Clipboard::ReadAsciiText(ClipboardType type, std::string* result) const {
+void Clipboard::ReadAsciiText(Clipboard::Buffer buffer,
+                              std::string* result) const {
   DCHECK(CalledOnValidThread());
-  DCHECK_EQ(type, CLIPBOARD_TYPE_COPY_PASTE);
+  DCHECK_EQ(buffer, BUFFER_STANDARD);
   *result = g_map.Get().Get(kPlainTextFormat);
 }
 
 // Note: |src_url| isn't really used. It is only implemented in Windows
-void Clipboard::ReadHTML(ClipboardType type,
+void Clipboard::ReadHTML(Clipboard::Buffer buffer,
                          string16* markup,
                          std::string* src_url,
                          uint32* fragment_start,
                          uint32* fragment_end) const {
   DCHECK(CalledOnValidThread());
-  DCHECK_EQ(type, CLIPBOARD_TYPE_COPY_PASTE);
+  DCHECK_EQ(buffer, BUFFER_STANDARD);
   if (src_url)
     src_url->clear();
 
@@ -280,14 +276,14 @@ void Clipboard::ReadHTML(ClipboardType type,
   *fragment_end = static_cast<uint32>(markup->length());
 }
 
-void Clipboard::ReadRTF(ClipboardType type, std::string* result) const {
+void Clipboard::ReadRTF(Buffer buffer, std::string* result) const {
   DCHECK(CalledOnValidThread());
   NOTIMPLEMENTED();
 }
 
-SkBitmap Clipboard::ReadImage(ClipboardType type) const {
+SkBitmap Clipboard::ReadImage(Buffer buffer) const {
   DCHECK(CalledOnValidThread());
-  DCHECK_EQ(type, CLIPBOARD_TYPE_COPY_PASTE);
+  DCHECK_EQ(buffer, BUFFER_STANDARD);
   std::string input = g_map.Get().Get(kBitmapFormat);
 
   SkBitmap bmp;
@@ -307,7 +303,7 @@ SkBitmap Clipboard::ReadImage(ClipboardType type) const {
   return bmp;
 }
 
-void Clipboard::ReadCustomData(ClipboardType clipboard_type,
+void Clipboard::ReadCustomData(Buffer buffer,
                                const string16& type,
                                string16* result) const {
   DCHECK(CalledOnValidThread());