GTK: Final experiment with Unity Global Menus before I give up on icons.

If you previously had issues with bad lag on Ubuntu Natty, please try this build with the command line flag "--enable-icons-in-global-history-menu". If this both lets icons be in the history menu and makes chrome startup instant, we can roll this out by default.

Revert my patch that just removed icons all together and insert some defensive
coding to check the output of the favicon database. If an icon is larger than
128 pixels, assume corruption and fallback to the default icon. If it's larger
than 16x16, resize it down to 16x16.

This reverts commit 8e01e5a72bfe1e9793f5e7986e984ec7c146c445 (r91615).

BUG=86715
TEST=none


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

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

4 files changed, 146 insertions, 4 deletions

diff --git a/chrome/browser/ui/gtk/global_history_menu.cc b/chrome/browser/ui/gtk/global_history_menu.cc
index 6ac4dd8..5b2617c 100644
--- a/chrome/browser/ui/gtk/global_history_menu.cc
+++ b/chrome/browser/ui/gtk/global_history_menu.cc
@@ -6,9 +6,11 @@
 
 #include <gtk/gtk.h>
 
+#include "base/command_line.h"
 #include "base/stl_util.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/tab_restore_service.h"
@@ -21,6 +23,7 @@
 #include "chrome/browser/ui/gtk/gtk_util.h"
 #include "chrome/browser/ui/gtk/owned_widget_gtk.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
 #include "content/common/notification_service.h"
 #include "grit/generated_resources.h"
@@ -28,6 +31,8 @@
 #include "ui/base/text/text_elider.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/gtk_util.h"
+#include "skia/ext/image_operations.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 
 namespace {
 
@@ -57,7 +62,9 @@ struct GlobalHistoryMenu::GetIndexClosure {
 class GlobalHistoryMenu::HistoryItem {
  public:
   HistoryItem()
-      : menu_item(NULL),
+      : icon_requested(false),
+        icon_handle(0),
+        menu_item(NULL),
         session_id(0) {}
 
   // The title for the menu item.
@@ -65,6 +72,16 @@ class GlobalHistoryMenu::HistoryItem {
   // The URL that will be navigated to if the user selects this item.
   GURL url;
 
+  // If the icon is being requested from the FaviconService, |icon_requested|
+  // will be true and |icon_handle| will be non-NULL. If this is false, then
+  // |icon_handle| will be NULL.
+  bool icon_requested;
+  // The Handle given to us by the FaviconService for the icon fetch request.
+  FaviconService::Handle icon_handle;
+
+  // The icon as a GtkImage for inclusion in a GtkImageMenuItem.
+  OwnedWidgetGtk icon_image;
+
   // A pointer to the menu_item. This is a weak reference in the GTK+ version
   // because the GtkMenu must sink the reference.
   GtkWidget* menu_item;
@@ -89,8 +106,12 @@ class GlobalHistoryMenu::HistoryItem {
 GlobalHistoryMenu::GlobalHistoryMenu(Browser* browser)
     : browser_(browser),
       profile_(browser_->profile()),
+      icon_experiment_on_(CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableIconsInGlobalHistoryMenu)),
       top_sites_(NULL),
+      default_favicon_(NULL),
       tab_restore_service_(NULL) {
+
 }
 
 GlobalHistoryMenu::~GlobalHistoryMenu() {
@@ -112,6 +133,8 @@ void GlobalHistoryMenu::Init(GtkWidget* history_menu,
   g_signal_connect(history_menu_item, "activate",
                    G_CALLBACK(OnMenuActivateThunk), this);
 
+  default_favicon_ = GtkThemeService::GetDefaultFavicon(true);
+
   if (profile_) {
     top_sites_ = profile_->GetTopSites();
     if (top_sites_) {
@@ -122,6 +145,10 @@ void GlobalHistoryMenu::Init(GtkWidget* history_menu,
       registrar_.Add(this, chrome::NOTIFICATION_TOP_SITES_CHANGED,
                      Source<history::TopSites>(top_sites_));
     }
+
+    registrar_.Add(this, chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
+                   Source<ThemeService>(
+                       ThemeServiceFactory::GetForProfile(profile_)));
   }
 }
 
@@ -150,6 +177,10 @@ void GlobalHistoryMenu::OnTopSitesReceived(
     item->title = visited.title;
     item->url = visited.url;
 
+    // The TopSites system doesn't give us icons; it gives us chrome:// urls
+    // to icons so fetch the icons normally.
+    GetFaviconForHistoryItem(item);
+
     AddHistoryItemToMenu(item,
                          history_menu_.get(),
                          GlobalMenuBar::TAG_MOST_VISITED,
@@ -188,6 +219,9 @@ GlobalHistoryMenu::HistoryItem* GlobalHistoryMenu::HistoryItemForTab(
   item->url = current_navigation.virtual_url();
   item->session_id = entry.id;
 
+  // Tab navigations don't come with icons, so we always have to request them.
+  GetFaviconForHistoryItem(item);
+
   return item;
 }
 
@@ -202,7 +236,7 @@ GtkWidget* GlobalHistoryMenu::AddHistoryItemToMenu(HistoryItem* item,
     title = UTF8ToUTF16(url_string);
   ui::ElideString(title, kMaximumMenuWidthInChars, &title);
 
-  GtkWidget* menu_item = gtk_menu_item_new_with_label(
+  GtkWidget* menu_item = gtk_image_menu_item_new_with_label(
       UTF16ToUTF8(title).c_str());
 
   item->menu_item = menu_item;
@@ -210,6 +244,17 @@ GtkWidget* GlobalHistoryMenu::AddHistoryItemToMenu(HistoryItem* item,
   g_object_set_data(G_OBJECT(menu_item), "type-tag", GINT_TO_POINTER(tag));
   g_signal_connect(menu_item, "activate",
                    G_CALLBACK(OnRecentlyClosedItemActivatedThunk), this);
+  if (icon_experiment_on_) {
+    gtk_util::SetAlwaysShowImage(menu_item);
+    if (item->icon_image.get()) {
+      gtk_image_menu_item_set_image(GTK_IMAGE_MENU_ITEM(menu_item),
+                                    item->icon_image.get());
+    } else if (!item->tabs.size()) {
+      gtk_image_menu_item_set_image(
+          GTK_IMAGE_MENU_ITEM(menu_item),
+          gtk_image_new_from_pixbuf(default_favicon_));
+    }
+  }
 
   std::string tooltip = gtk_util::BuildTooltipTitleFor(item->title, item->url);
   gtk_widget_set_tooltip_markup(menu_item, tooltip.c_str());
@@ -220,6 +265,71 @@ GtkWidget* GlobalHistoryMenu::AddHistoryItemToMenu(HistoryItem* item,
   return menu_item;
 }
 
+void GlobalHistoryMenu::GetFaviconForHistoryItem(HistoryItem* item) {
+  if (icon_experiment_on_) {
+    FaviconService* service =
+        profile_->GetFaviconService(Profile::EXPLICIT_ACCESS);
+    FaviconService::Handle handle = service->GetFaviconForURL(
+        item->url,
+        history::FAVICON,
+        &favicon_consumer_,
+        NewCallback(this, &GlobalHistoryMenu::GotFaviconData));
+    favicon_consumer_.SetClientData(service, handle, item);
+    item->icon_handle = handle;
+    item->icon_requested = true;
+  }
+}
+
+void GlobalHistoryMenu::GotFaviconData(FaviconService::Handle handle,
+                                       history::FaviconData favicon) {
+  HistoryItem* item =
+      favicon_consumer_.GetClientData(
+          profile_->GetFaviconService(Profile::EXPLICIT_ACCESS), handle);
+  DCHECK(item);
+  item->icon_requested = false;
+  item->icon_handle = static_cast<CancelableRequestProvider::Handle>(NULL);
+
+  SkBitmap icon;
+  if (favicon.is_valid() &&
+      gfx::PNGCodec::Decode(favicon.image_data->front(),
+                            favicon.image_data->size(), &icon)) {
+    // We're going to assume anything larger than 128x128 is a mistake and the
+    // database is incorrect.
+    if (icon.width() > 128 || icon.height() > 128) {
+      return;
+    } else if (icon.width() > 16 || icon.height() > 16) {
+      // But if we have an icon that's a little big, we want to resize it down
+      // to 16x16 before we stuff it in the gtk menu because this will be sent
+      // over dbus.
+      icon = skia::ImageOperations::Resize(
+          icon, skia::ImageOperations::RESIZE_GOOD, 16, 16);
+    }
+
+    GdkPixbuf* pixbuf = gfx::GdkPixbufFromSkBitmap(&icon);
+    if (pixbuf) {
+      item->icon_image.Own(gtk_image_new_from_pixbuf(pixbuf));
+      g_object_unref(pixbuf);
+
+      if (item->menu_item) {
+        gtk_image_menu_item_set_image(
+            GTK_IMAGE_MENU_ITEM(item->menu_item),
+            item->icon_image.get());
+      }
+    }
+  }
+}
+
+void GlobalHistoryMenu::CancelFaviconRequest(HistoryItem* item) {
+  DCHECK(item);
+  if (item->icon_requested) {
+    FaviconService* service =
+        profile_->GetFaviconService(Profile::EXPLICIT_ACCESS);
+    service->CancelRequest(item->icon_handle);
+    item->icon_requested = false;
+    item->icon_handle = static_cast<CancelableRequestProvider::Handle>(NULL);
+  }
+}
+
 int GlobalHistoryMenu::GetIndexOfMenuItemWithTag(GtkWidget* menu, int tag_id) {
   GetIndexClosure closure;
   closure.found = false;
@@ -267,6 +377,7 @@ void GlobalHistoryMenu::ClearMenuCallback(GtkWidget* menu_item,
     HistoryItem* item = closure->menu_bar->HistoryItemForMenuItem(menu_item);
 
     if (item) {
+      closure->menu_bar->CancelFaviconRequest(item);
       closure->menu_bar->menu_item_history_map_.erase(menu_item);
       delete item;
     }
@@ -282,7 +393,12 @@ void GlobalHistoryMenu::ClearMenuCallback(GtkWidget* menu_item,
 void GlobalHistoryMenu::Observe(int type,
                                 const NotificationSource& source,
                                 const NotificationDetails& details) {
-  if (type == chrome::NOTIFICATION_TOP_SITES_CHANGED) {
+  if (type == chrome::NOTIFICATION_BROWSER_THEME_CHANGED) {
+    // Keeping track of which menu items have the default icon is going an
+    // error-prone pain, so instead just store the new default favicon and
+    // we'll update on the next menu change event.
+    default_favicon_ = GtkThemeService::GetDefaultFavicon(true);
+  } else if (type == chrome::NOTIFICATION_TOP_SITES_CHANGED) {
     if (Source<history::TopSites>(source).ptr() == top_sites_)
       GetTopSitesData();
   } else {
@@ -379,8 +495,9 @@ void GlobalHistoryMenu::TabRestoreServiceChanged(TabRestoreService* service) {
               base::IntToString16(item->tabs.size()));
 
       // Create the menu item parent. Unlike mac, it's can't be activated.
-      GtkWidget* parent_item = gtk_menu_item_new_with_label(
+      GtkWidget* parent_item = gtk_image_menu_item_new_with_label(
           title.c_str());
+      gtk_util::SetAlwaysShowImage(parent_item);
       gtk_widget_show(parent_item);
       g_object_set_data(G_OBJECT(parent_item), "type-tag",
                         GINT_TO_POINTER(GlobalMenuBar::TAG_RECENTLY_CLOSED));
diff --git a/chrome/browser/ui/gtk/global_history_menu.h b/chrome/browser/ui/gtk/global_history_menu.h
index 9ddf74e..f81ca83 100644
--- a/chrome/browser/ui/gtk/global_history_menu.h
+++ b/chrome/browser/ui/gtk/global_history_menu.h
@@ -7,6 +7,7 @@
 
 #include <map>
 
+#include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/sessions/tab_restore_service.h"
 #include "chrome/browser/sessions/tab_restore_service_observer.h"
@@ -66,6 +67,17 @@ class GlobalHistoryMenu : public GlobalMenuOwner,
                                   int tag,
                                   int index);
 
+  // Requests a FavIcon; we'll receive the data in the future through the
+  // GotFaviconData() callback.
+  void GetFaviconForHistoryItem(HistoryItem* item);
+
+  // Callback for GetFaviconForHistoryItem().
+  void GotFaviconData(FaviconService::Handle handle,
+                      history::FaviconData favicon);
+
+  // Cancels an outstanding favicon request.
+  void CancelFaviconRequest(HistoryItem* item);
+
   // Find the first index of the item in |menu| with the tag |tag_id|.
   int GetIndexOfMenuItemWithTag(GtkWidget* menu, int tag_id);
 
@@ -102,6 +114,8 @@ class GlobalHistoryMenu : public GlobalMenuOwner,
   Browser* browser_;
   Profile* profile_;
 
+  bool icon_experiment_on_;
+
   // The history menu. We keep this since we need to rewrite parts of it
   // periodically.
   OwnedWidgetGtk history_menu_;
@@ -109,11 +123,16 @@ class GlobalHistoryMenu : public GlobalMenuOwner,
   history::TopSites* top_sites_;
   CancelableRequestConsumer top_sites_consumer_;
 
+  GdkPixbuf* default_favicon_;
+
   TabRestoreService* tab_restore_service_;  // weak
 
   // A mapping from GtkMenuItems to HistoryItems that maintain data.
   MenuItemToHistoryMap menu_item_history_map_;
 
+  // Maps HistoryItems to favicon request Handles.
+  CancelableRequestConsumerTSimple<HistoryItem*> favicon_consumer_;
+
   NotificationRegistrar registrar_;
 };
 
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 14d3941..95d4831 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -1169,6 +1169,11 @@ const char kKioskMode[]                     = "kiosk";
 // minute because it doesn't like it when we thrown hundreds of kilobytes (or
 // even megabytes) of favicon data at it.
 const char kEnableGlobalBookmarkMenu[]     = "enable-global-bookmark-menu";
+
+// An experiment to see if it's corrupted favicon data that's making unity
+// global menu bar startup slow.
+const char kEnableIconsInGlobalHistoryMenu[] =
+    "enable-icons-in-global-history-menu";
 #endif
 
 #if defined(TOOLKIT_VIEWS)
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 3d9aebf..54b36a2 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -339,6 +339,7 @@ extern const char kKioskMode[];
 
 #if defined(TOOLKIT_GTK)
 extern const char kEnableGlobalBookmarkMenu[];
+extern const char kEnableIconsInGlobalHistoryMenu[];
 #endif
 
 #if defined(TOOLKIT_VIEWS)