Improve large tab strip by leveraging touch icons when present

chrome/browser/ui/touch/tabs/touch_tab_strip_controller.h

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_TOUCH_TABS_TOUCH_TAB_STRIP_CONTROLLER_H_
+#define CHROME_BROWSER_UI_TOUCH_TABS_TOUCH_TAB_STRIP_CONTROLLER_H_
+#pragma once
+
+#include <map>
+
+#include "chrome/browser/favicon/favicon_service.h"
+#include "chrome/browser/history/history.h"
+#include "chrome/browser/ui/views/tabs/browser_tab_strip_controller.h"
+
+class Profile;
+
+// Subclassing BrowserTabStripController to provide touch specific behavior.
+class TouchTabStripController: public BrowserTabStripController {
+ public:
+  TouchTabStripController(Browser* browser, TabStripModel* model);
+  virtual ~TouchTabStripController();
+
+  virtual void TabChangedAt(TabContentsWrapper* contents, int model_index,

[sky, 2011/06/09 16:53:00]

nit: if you have to wrap, then put each param on its own line and align with the
openning '('.

[Emmanuel Saint-loubert-Bié, 2011/06/10 02:21:45]

Done.

+      TabChangeType change_type) OVERRIDE;
+
+  void OnTouchIconAvailable(FaviconService::Handle h,

[sky, 2011/06/09 16:53:00]

make this private.

[Emmanuel Saint-loubert-Bié, 2011/06/10 02:21:45]

Done.

+      history::FaviconData favicon);
+
+ protected:
+  virtual void SetTabRendererDataFromModel(TabContents* contents,
+      int model_index, TabRendererData* data) OVERRIDE;
+
+ private:
+

[sky, 2011/06/09 16:53:00]

nit: no newline here.

[Emmanuel Saint-loubert-Bié, 2011/06/10 02:21:45]

Done.

+  // Profile obtained from Browser object in constructor.
+  Profile* profile_;

[sky, 2011/06/09 16:53:00]

Remove this and make profile() in BrowserTabStripController protected.

[Emmanuel Saint-loubert-Bié, 2011/06/10 02:21:45]

Cool and done.

+
+  // Mapping of model index to URLS and URLs to Touch Icons
+  typedef std::map<int, GURL> ModelIndexMap;

[sky, 2011/06/09 16:53:00]

If you cache by model index it means you have to track moves and deletes.
Additionally the Tab already has the url and icon. You should do the following:
. Make SetTabRendererDataFromModel take a boolean indicating if the tab is being
newly inserted.
. In TouchTabStripController if the tab isn't being inserted you know you can
get the TabRendererData from the existing tab to see if you can reuse the
existing bitmap, or if you need to do a new load.

Then you need to figure out how to resolve what tab an icon being returned from
OnTouchIconAvailable corresponds to. Maybe you could add an ever increasing
integer to TouchTab for that.

When doing url comparison you should ignore everything after the fragment.

[Emmanuel Saint-loubert-Bié, 2011/06/10 02:21:45]

You are right of course using the model that way is not ideal. I have changed
the code to a vector of requests with the index of the request passed to the
callback. That way I know for sure which GURL need to receive the bitmap in the 
callback. I keep the model_index around just to be used on the tab refresh. If
we are unlucky then we might not refresh the right tab, but that is probably not
a big deal?

Finally I now use GURL::GetWithEmptyPath() for the comparison this should ignore
the fragment from what I understand, but let me know your thoughts.

+  ModelIndexMap model_index_map_;
+  typedef std::map<GURL, SkBitmap> TouchIconMap;
+  TouchIconMap touch_icon_map_;
+
+  // Our consumer for touch icon requests (takes the model_index as data)
+  CancelableRequestConsumerT<int, 0> consumer_;
+
+  DISALLOW_COPY_AND_ASSIGN(TouchTabStripController);
+};
+
+#endif  // CHROME_BROWSER_UI_TOUCH_TABS_TOUCH_TAB_STRIP_CONTROLLER_H_