GTK: implement bookmark manager copy/paste.

- Refactor bookmark util clipboard code to be cross platform.
- Add a CBF_DATA format for writing arbitrary data to the clipboard.
- Change MenuGtk so that showing a menu only makes the menu items update asynchronously. This way we can show the menu immediately and we won't get in trouble if the delegate's IsCommandEnabled() runs a nested message loop.

I did not do any mac-side implementation of the new clipboard stuff. The implementation should be easy when they get around to writing the bookmark manager.

Review URL: http://codereview.chromium.org/159815

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@22419 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 104 insertions, 50 deletions

diff --git a/chrome/browser/bookmarks/bookmark_drag_data.cc b/chrome/browser/bookmarks/bookmark_drag_data.cc
index bf3ae05..0ed2273 100644
--- a/chrome/browser/bookmarks/bookmark_drag_data.cc
+++ b/chrome/browser/bookmarks/bookmark_drag_data.cc
@@ -4,25 +4,33 @@
 
 #include "chrome/browser/bookmarks/bookmark_drag_data.h"
 
-// TODO(port): Port this file.
-#if defined(TOOLKIT_VIEWS)
-#include "app/os_exchange_data.h"
-#else
-#include "chrome/common/temp_scaffolding_stubs.h"
-#endif
 #include "base/basictypes.h"
 #include "base/pickle.h"
+#include "base/scoped_clipboard_writer.h"
 #include "base/string_util.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/profile.h"
 #include "chrome/common/url_constants.h"
+#include "chrome/browser/browser_process.h"
+
+// TODO(port): Port this file.
+#if defined(TOOLKIT_VIEWS)
+#include "app/os_exchange_data.h"
+#else
+#include "chrome/common/temp_scaffolding_stubs.h"
+#endif
+
+const char* BookmarkDragData::kClipboardFormatString =
+    "chromium/x-bookmark-entries";
 
 #if defined(OS_WIN)
 static CLIPFORMAT clipboard_format = 0;
 
 static void RegisterFormat() {
   if (clipboard_format == 0) {
-    clipboard_format = RegisterClipboardFormat(L"chrome/x-bookmark-entries");
+    clipboard_format =
+        ::RegisterClipboardFormat(ASCIIToWide(
+            BookmarkDragData::kClipboardFormatString).c_str());
     DCHECK(clipboard_format);
   }
 }
@@ -86,6 +94,51 @@ BookmarkDragData::BookmarkDragData(
     elements.push_back(Element(nodes[i]));
 }
 
+#if !defined(OS_MACOSX)
+void BookmarkDragData::WriteToClipboard(Profile* profile) const {
+  ScopedClipboardWriter scw(g_browser_process->clipboard());
+
+  // If there is only one element and it is a URL, write the URL to the
+  // clipboard.
+  if (elements.size() == 1 && elements[0].is_url) {
+    scw.WriteBookmark(WideToUTF16Hack(elements[0].title),
+                      elements[0].url.spec());
+  }
+
+  Pickle pickle;
+  WriteToPickle(profile, &pickle);
+  scw.WritePickledData(pickle, kClipboardFormatString);
+}
+
+bool BookmarkDragData::ReadFromClipboard() {
+  std::string data;
+  Clipboard* clipboard = g_browser_process->clipboard();
+  clipboard->ReadData(kClipboardFormatString, &data);
+
+  if (!data.empty()) {
+    Pickle pickle(data.data(), data.size());
+    if (ReadFromPickle(&pickle))
+      return true;
+  }
+
+  string16 title;
+  std::string url;
+  clipboard->ReadBookmark(&title, &url);
+  if (!url.empty()) {
+    Element element;
+    element.is_url = true;
+    element.url = GURL(url);
+    element.title = UTF16ToWideHack(title);
+
+    elements.clear();
+    elements.push_back(element);
+    return true;
+  }
+
+  return false;
+}
+#endif  // !defined(OS_MACOSX)
+
 #if defined(OS_WIN)
 void BookmarkDragData::Write(Profile* profile, OSExchangeData* data) const {
   RegisterFormat();
diff --git a/chrome/browser/bookmarks/bookmark_drag_data.h b/chrome/browser/bookmarks/bookmark_drag_data.h
index 9af3c20..815e8e1d 100644
--- a/chrome/browser/bookmarks/bookmark_drag_data.h
+++ b/chrome/browser/bookmarks/bookmark_drag_data.h
@@ -71,6 +71,17 @@ struct BookmarkDragData {
   explicit BookmarkDragData(const BookmarkNode* node);
   explicit BookmarkDragData(const std::vector<const BookmarkNode*>& nodes);
 
+  // TODO(estade): Port to mac. It should be as simple as removing this ifdef
+  // after the relevant Clipboard functions are implemetned.
+#if !defined(OS_MACOSX)
+  // Writes elements to the clipboard.
+  void WriteToClipboard(Profile* profile) const;
+
+  // Reads bookmarks from the clipboard. Prefers data written via
+  // WriteToClipboard but will also attempt to read a plain bookmark.
+  bool ReadFromClipboard();
+#endif
+
 #if defined(TOOLKIT_VIEWS)
   // Writes elements to data. If there is only one element and it is a URL
   // the URL and title are written to the clipboard in a format other apps can
@@ -114,6 +125,9 @@ struct BookmarkDragData {
   // The actual elements written to the clipboard.
   std::vector<Element> elements;
 
+  // The MIME type for the clipboard format for BookmarkDragData.
+  static const char* kClipboardFormatString;
+
  private:
   // Path of the profile we originated from.
 #if defined(WCHAR_T_IS_UTF16)
diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc
index 7371497..51250c3 100644
--- a/chrome/browser/bookmarks/bookmark_utils.cc
+++ b/chrome/browser/bookmarks/bookmark_utils.cc
@@ -6,20 +6,16 @@
 
 #include "app/drag_drop_types.h"
 #include "app/l10n_util.h"
-// TODO(port): Port these files.
-#if defined(OS_WIN)
-#include "app/os_exchange_data.h"
-#else
-#include "chrome/common/temp_scaffolding_stubs.h"
-#endif
 #include "app/tree_node_iterator.h"
 #include "base/basictypes.h"
+#include "base/clipboard.h"
 #include "base/string_util.h"
 #include "base/time.h"
 #include "chrome/browser/bookmarks/bookmark_drag_data.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_list.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_window.h"
 #include "chrome/browser/history/query_parser.h"
 #include "chrome/browser/profile.h"
@@ -355,53 +351,39 @@ void OpenAll(gfx::NativeView parent,
 void CopyToClipboard(BookmarkModel* model,
                      const std::vector<const BookmarkNode*>& nodes,
                      bool remove_nodes) {
+#if defined(OS_WIN) || defined(OS_LINUX)
   if (nodes.empty())
     return;
 
-#if defined(OS_WIN)
-  OSExchangeData* data = new OSExchangeData();
-  BookmarkDragData(nodes).Write(NULL, data);
-  OleSetClipboard(data);
-  // OLE takes ownership of OSExchangeData.
-#else
-  // TODO(port): Clipboard integration.  Don't we have clipboard
-  // implemented somewhere else?
-#endif
+  BookmarkDragData(nodes).WriteToClipboard(NULL);
 
-  // The following works cross-platform but I'm disabling it for now so that
-  // users who Cut a bookmark or folder won't lose it without being able to
-  // paste it.
-#if defined(OS_WIN)
   if (remove_nodes) {
     for (size_t i = 0; i < nodes.size(); ++i) {
       model->Remove(nodes[i]->GetParent(),
                     nodes[i]->GetParent()->IndexOfChild(nodes[i]));
     }
   }
+#else
+  // Not implemented on mac yet.
 #endif
 }
 
 void PasteFromClipboard(BookmarkModel* model,
                         const BookmarkNode* parent,
                         int index) {
+#if defined(OS_WIN) || defined(OS_LINUX)
   if (!parent)
     return;
 
-#if defined(OS_WIN)
-  IDataObject* data;
-  if (OleGetClipboard(&data) != S_OK)
-    return;
-
-  OSExchangeData data_wrapper(data);
   BookmarkDragData bookmark_data;
-  if (!bookmark_data.Read(data_wrapper))
+  if (!bookmark_data.ReadFromClipboard())
     return;
 
   if (index == -1)
     index = parent->GetChildCount();
   bookmark_utils::CloneDragData(model, bookmark_data.elements, parent, index);
 #else
-  // TODO(port): Clipboard integration.
+  // Not implemented on mac yet.
 #endif
 }
 
@@ -409,18 +391,8 @@ bool CanPasteFromClipboard(const BookmarkNode* node) {
   if (!node)
     return false;
 
-#if defined(OS_WIN)
-  IDataObject* data;
-  if (OleGetClipboard(&data) != S_OK)
-    return false;
-
-  OSExchangeData data_wrapper(data);
-  BookmarkDragData bookmark_data;
-  return bookmark_data.Read(data_wrapper);
-#else
-  // TODO(port): Clipboard integration.
-  return false;
-#endif
+  return g_browser_process->clipboard()->IsFormatAvailable(
+      BookmarkDragData::kClipboardFormatString);
 }
 
 std::string GetNameForURL(const GURL& url) {
diff --git a/chrome/browser/gtk/menu_gtk.cc b/chrome/browser/gtk/menu_gtk.cc
index 492bc97..73a3296 100644
--- a/chrome/browser/gtk/menu_gtk.cc
+++ b/chrome/browser/gtk/menu_gtk.cc
@@ -7,6 +7,7 @@
 #include "app/l10n_util.h"
 #include "base/gfx/gtk_util.h"
 #include "base/logging.h"
+#include "base/message_loop.h"
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "chrome/browser/gtk/standard_menus.h"
@@ -22,7 +23,8 @@ MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
                  GtkAccelGroup* accel_group)
     : delegate_(delegate),
       dummy_accel_group_(gtk_accel_group_new()),
-      menu_(gtk_menu_new()) {
+      menu_(gtk_menu_new()),
+      factory_(this) {
   ConnectSignalHandlers();
   BuildMenuIn(menu_.get(), menu_data, accel_group);
 }
@@ -30,7 +32,8 @@ MenuGtk::MenuGtk(MenuGtk::Delegate* delegate,
 MenuGtk::MenuGtk(MenuGtk::Delegate* delegate, bool load)
     : delegate_(delegate),
       dummy_accel_group_(NULL),
-      menu_(gtk_menu_new()) {
+      menu_(gtk_menu_new()),
+      factory_(this) {
   ConnectSignalHandlers();
   if (load)
     BuildMenuFromDelegate();
@@ -43,6 +46,8 @@ MenuGtk::~MenuGtk() {
 }
 
 void MenuGtk::ConnectSignalHandlers() {
+  // We connect afterwards because OnMenuShow calls SetMenuItemInfo, which may
+  // take a long time or even start a nested message loop.
   g_signal_connect(menu_.get(), "show", G_CALLBACK(OnMenuShow), this);
   g_signal_connect(menu_.get(), "hide", G_CALLBACK(OnMenuHidden), this);
 }
@@ -293,10 +298,14 @@ void MenuGtk::MenuPositionFunc(GtkMenu* menu,
   *push_in = FALSE;
 }
 
+void MenuGtk::UpdateMenu() {
+  gtk_container_foreach(GTK_CONTAINER(menu_.get()), SetMenuItemInfo, this);
+}
+
 // static
 void MenuGtk::OnMenuShow(GtkWidget* widget, MenuGtk* menu) {
-  gtk_container_foreach(GTK_CONTAINER(menu->menu_.get()),
-                        SetMenuItemInfo, menu);
+  MessageLoop::current()->PostTask(FROM_HERE,
+      menu->factory_.NewRunnableMethod(&MenuGtk::UpdateMenu));
 }
 
 // static
diff --git a/chrome/browser/gtk/menu_gtk.h b/chrome/browser/gtk/menu_gtk.h
index b06c77c..2d47f67 100644
--- a/chrome/browser/gtk/menu_gtk.h
+++ b/chrome/browser/gtk/menu_gtk.h
@@ -9,6 +9,7 @@
 
 #include <string>
 
+#include "base/task.h"
 #include "chrome/common/owned_widget_gtk.h"
 
 class SkBitmap;
@@ -114,6 +115,9 @@ class MenuGtk {
   // recursive and does not support sub-menus.
   void BuildMenuFromDelegate();
 
+  // Contains implementation for OnMenuShow.
+  void UpdateMenu();
+
   // Callback for when a menu item is clicked.
   static void OnMenuItemActivated(GtkMenuItem* menuitem, MenuGtk* menu);
 
@@ -142,6 +146,8 @@ class MenuGtk {
   // menu items from getting activated when we are setting up the
   // menu.
   static bool block_activation_;
+
+  ScopedRunnableMethodFactory<MenuGtk> factory_;
 };
 
 #endif  // CHROME_BROWSER_GTK_MENU_GTK_H_