Clear the clipboard closing Incognito window

ui/base/clipboard/clipboard_gtk.cc

Index: ui/base/clipboard/clipboard_gtk.cc
diff --git a/ui/base/clipboard/clipboard_gtk.cc b/ui/base/clipboard/clipboard_gtk.cc
index 7473f31097289eb0b7bc716711c256392a2fd88f..d14e35f4378d9b201d7018ede9390b2b11be3bfb 100644
--- a/ui/base/clipboard/clipboard_gtk.cc
+++ b/ui/base/clipboard/clipboard_gtk.cc
@@ -106,6 +106,7 @@ GdkFilterReturn SelectionChangeObserver::OnXEvent(GdkXEvent* xevent,
   return GDK_FILTER_CONTINUE;
 }
 
+const char kSourceTagType[] = "org.chromium.source-tag";
 const char kMimeTypeBitmap[] = "image/bmp";
 const char kMimeTypeMozillaURL[] = "text/x-moz-url";
 const char kMimeTypePepperCustomData[] = "chromium/x-pepper-custom-data";
@@ -152,7 +153,7 @@ void GetData(GtkClipboard* clipboard,
 // GtkClipboardClearFunc callback.
 // We are guaranteed this will be called exactly once for each call to
 // gtk_clipboard_set_with_data.
-void ClearData(GtkClipboard* clipboard,
+void ClearData(GtkClipboard* /*clipboard*/,
                gpointer user_data) {
   Clipboard::TargetMap* map =
       reinterpret_cast<Clipboard::TargetMap*>(user_data);
@@ -176,6 +177,36 @@ void ClearData(GtkClipboard* clipboard,
   delete map;
 }
 
+// Sets the content of |clipboard|. |clipboard_data| is to be allocated by new.
+// The ownership is transferred to the function. |clipboard_data| is destroyed
+// inside upon failure.

[battre, 2013/02/12 12:50:59]

I think the entire comment can be simplified to
// Sets the content of |clipboard| and takes ownership of |clipboard_data|.

[vasilii, 2013/02/12 15:58:32]

The whole function is reverted...

+bool GtkClipboardSetWithData(GtkClipboard* clipboard,
+                             Clipboard::TargetMap* clipboard_data) {
+  scoped_array<GtkTargetEntry> targets(
+      new GtkTargetEntry[clipboard_data->size()]);
+
+  int i = 0;
+  for (Clipboard::TargetMap::iterator iter = clipboard_data->begin();
+      iter != clipboard_data->end(); ++iter, ++i) {

[battre, 2013/02/12 12:50:59]

nit: +1 space

[vasilii, 2013/02/12 15:58:32]

Done.

+    targets[i].target = const_cast<char*>(iter->first.c_str());
+    targets[i].flags = 0;
+    targets[i].info = 0;
+  }
+
+  if (gtk_clipboard_set_with_data(clipboard, targets.get(),
+                                  clipboard_data->size(),
+                                  GetData, ClearData,
+                                  clipboard_data)) {
+    gtk_clipboard_set_can_store(clipboard,
+                                targets.get(),
+                                clipboard_data->size());
+    return true;
+  }
+  // We should clear data manually on failure.
+  ClearData(clipboard, clipboard_data);
+  return false;
+}
+
 }  // namespace
 
 Clipboard::FormatType::FormatType() : data_(GDK_NONE) {
@@ -217,7 +248,9 @@ Clipboard::~Clipboard() {
   gtk_clipboard_store(clipboard_);
 }
 
-void Clipboard::WriteObjects(Buffer buffer, const ObjectMap& objects) {
+void Clipboard::WriteObjectsImpl(Buffer buffer,
+                                 const ObjectMap& objects,
+                                 SourceTag tag) {
   DCHECK(CalledOnValidThread());
   clipboard_data_ = new TargetMap();
 
@@ -225,41 +258,25 @@ void Clipboard::WriteObjects(Buffer buffer, const ObjectMap& objects) {
        iter != objects.end(); ++iter) {
     DispatchObject(static_cast<ObjectType>(iter->first), iter->second);
   }
+  WriteSourceTag(tag);
 
-  SetGtkClipboard(buffer);
+  SetGtkClipboard(buffer, tag);

[dcheng, 2013/02/11 22:58:21]

I'd prefer not to pass tag explicitly here. It's already in clipboard_data_
right?

[vasilii, 2013/02/12 15:58:32]

Done.

 }
 
 // Take ownership of the GTK clipboard and inform it of the targets we support.
-void Clipboard::SetGtkClipboard(Buffer buffer) {
-  scoped_ptr<GtkTargetEntry[]> targets(
-      new GtkTargetEntry[clipboard_data_->size()]);
-
-  int i = 0;
-  for (Clipboard::TargetMap::iterator iter = clipboard_data_->begin();
-       iter != clipboard_data_->end(); ++iter, ++i) {
-    targets[i].target = const_cast<char*>(iter->first.c_str());
-    targets[i].flags = 0;
-    targets[i].info = 0;
-  }
-
+void Clipboard::SetGtkClipboard(Buffer buffer, SourceTag tag) {
   GtkClipboard *clipboard = LookupBackingClipboard(buffer);
-
-  if (gtk_clipboard_set_with_data(clipboard, targets.get(),
-                                  clipboard_data_->size(),
-                                  GetData, ClearData,
-                                  clipboard_data_)) {
-    gtk_clipboard_set_can_store(clipboard,
-                                targets.get(),
-                                clipboard_data_->size());
-  }
-
-  if (buffer == BUFFER_STANDARD) {
-    Clipboard::TargetMap::iterator text_iter = clipboard_data_->find("TEXT");
-    if (text_iter != clipboard_data_->end()) {
-      gtk_clipboard_set_text(primary_selection_, text_iter->second.first,
-                             text_iter->second.second);
+  if (GtkClipboardSetWithData(clipboard, clipboard_data_))

[battre, 2013/02/12 12:50:59]

please add {}

[vasilii, 2013/02/12 15:58:32]

Done.

+    if (buffer == BUFFER_STANDARD) {
+      Clipboard::TargetMap::iterator text_iter = clipboard_data_->find("TEXT");
+      if (text_iter != clipboard_data_->end()) {
+        // Copy text and SourceTag to the selection clipboard.
+        clipboard_data_ = new TargetMap;

[dcheng, 2013/02/11 22:58:21]

I think this might be cleaner if you pull this logic up into WriteObjectsImpl.
It's a little icky to mess with clipboard_data_ here, even though it's
technically "OK", and that way, we won't have to plumb tag-specific code in
here. I'm thinking something like:

void Clipboard::WriteObjectsImpl(...) {
  // Current implementation here...
  SetGtkClipboard(buffer);
  if (/* clipboard has text and is the standard buffer */) {
    ObjectMap selection_objects;
    selection_objects.insert(/* iterator that points to text entry */);
    WriteObjectsImpl(BUFFER_SELECTION, selection_objects, tag);
  }
}

WDYT?

[vasilii, 2013/02/12 15:58:32]

Done.

+        WriteText(text_iter->second.first, text_iter->second.second);
+        WriteSourceTag(tag);
+        GtkClipboardSetWithData(primary_selection_, clipboard_data_);
+      }
     }
-  }
 
   // clipboard_data_ now owned by the GtkClipboard.
   clipboard_data_ = NULL;
@@ -350,6 +367,13 @@ void Clipboard::WriteData(const FormatType& format,
   InsertMapping(GdkAtomToString(format.ToGdkAtom()).c_str(), data, data_len);
 }
 
+void Clipboard::WriteSourceTag(SourceTag tag) {
+  if (tag != SourceTag()) {
+    ObjectMapParam binary = SourceTag2Binary(tag);
+    WriteData(GetSourceTagFormatType(), &binary[0], binary.size());
+  }
+}
+
 // We do not use gtk_clipboard_wait_is_target_available because of
 // a bug with the gtk clipboard. It caches the available targets
 // and does not always refresh the cache when it is appropriate.
@@ -576,9 +600,17 @@ void Clipboard::ReadBookmark(string16* title, std::string* url) const {
 }
 
 void Clipboard::ReadData(const FormatType& format, std::string* result) const {
+  ReadDataImpl(BUFFER_STANDARD, format, result);
+}
+
+void Clipboard::ReadDataImpl(Buffer buffer,
+                             const FormatType& format,
+                             std::string* result) const {
   DCHECK(CalledOnValidThread());
+  result->clear();
+  GtkClipboard* clipboard = LookupBackingClipboard(buffer);
   GtkSelectionData* data =
-      gtk_clipboard_wait_for_contents(clipboard_, format.ToGdkAtom());
+      gtk_clipboard_wait_for_contents(clipboard, format.ToGdkAtom());
   if (!data)
     return;
   result->assign(reinterpret_cast<const char*>(
@@ -587,6 +619,12 @@ void Clipboard::ReadData(const FormatType& format, std::string* result) const {
   gtk_selection_data_free(data);
 }
 
+Clipboard::SourceTag Clipboard::ReadSourceTag(Buffer buffer) const {
+  std::string result;
+  ReadDataImpl(buffer, GetSourceTagFormatType(), &result);
+  return Binary2SourceTag(result);
+}
+
 uint64 Clipboard::GetSequenceNumber(Buffer buffer) {
   DCHECK(CalledOnValidThread());
   if (buffer == BUFFER_STANDARD)
@@ -659,6 +697,12 @@ const Clipboard::FormatType& Clipboard::GetPepperCustomDataFormatType() {
   return type;
 }
 
+// static
+const Clipboard::FormatType& Clipboard::GetSourceTagFormatType() {
+  CR_DEFINE_STATIC_LOCAL(FormatType, type, (kSourceTagType));
+  return type;
+}
+
 void Clipboard::InsertMapping(const char* key,
                               char* data,
                               size_t data_len) {