Adds the ability to sort TableView. Contrary to what we spoke about

the other day I ended up doing the sorting in tableview. This makes it
a heck of lot easier than having every model have to deal with it. As
part of this I removed the optional non-caching logic from TableView,
which was never used. Sadly though, this means there are coordinate
transformations.

I've only enabled sorting in the keyword editor, I have to make sure
all the other places that use TableView can deal with it. For example,
task manager can't deal with it currently as it expects the getters to
be called only once where as when sorting they may be called multiple
times.

BUG=2790
TEST=This enables sorting ONLY in the keyword editor. Make sure there
  aren't any problems in adding/removing/changing entries in the
  keyword editor after this.

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

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

9 files changed, 978 insertions, 161 deletions

diff --git a/chrome/SConscript.unit_tests b/chrome/SConscript.unit_tests
index afd6eaa..98d80e6 100644
--- a/chrome/SConscript.unit_tests
+++ b/chrome/SConscript.unit_tests
@@ -233,6 +233,7 @@ if env_test['PLATFORM'] == 'win32':
       'renderer/spellcheck_unittest.cc',
       'views/focus_manager_unittest.cc',
       'views/grid_layout_unittest.cc',
+      'views/table_view_unittest.cc',
       'views/view_unittest.cc',
       'test/test_notification_tracker.cc',
       'test/testing_profile.cc',
diff --git a/chrome/browser/views/keyword_editor_view.cc b/chrome/browser/views/keyword_editor_view.cc
index a4e446d..9ef5527 100644
--- a/chrome/browser/views/keyword_editor_view.cc
+++ b/chrome/browser/views/keyword_editor_view.cc
@@ -471,12 +471,20 @@ void KeywordEditorView::Init() {
   columns.push_back(
       TableColumn(IDS_SEARCH_ENGINES_EDITOR_DESCRIPTION_COLUMN,
                   TableColumn::LEFT, -1, .75));
+  columns.back().sortable = true;
   columns.push_back(
       TableColumn(IDS_SEARCH_ENGINES_EDITOR_KEYWORD_COLUMN,
                   TableColumn::LEFT, -1, .25));
+  columns.back().sortable = true;
   table_view_ = new ChromeViews::TableView(table_model_.get(), columns,
       ChromeViews::ICON_AND_TEXT, false, true, true);
   table_view_->SetObserver(this);
+  // Make the table initially sorted by name.
+  ChromeViews::TableView::SortDescriptors sort;
+  sort.push_back(
+      ChromeViews::TableView::SortDescriptor(
+          IDS_SEARCH_ENGINES_EDITOR_DESCRIPTION_COLUMN, true));
+  table_view_->SetSortDescriptors(sort);
 
   add_button_ = new ChromeViews::NativeButton(
       l10n_util::GetString(IDS_SEARCH_ENGINES_EDITOR_NEW_BUTTON));
@@ -586,20 +594,20 @@ void KeywordEditorView::ButtonPressed(ChromeViews::NativeButton* sender) {
     // Remove the observer while we modify the model, that way we don't need to
     // worry about the model calling us back when we mutate it.
     url_model_->RemoveObserver(this);
-    int last_row = -1;
+    int last_view_row = -1;
     for (ChromeViews::TableView::iterator i = table_view_->SelectionBegin();
          i != table_view_->SelectionEnd(); ++i) {
-      last_row = *i;
-      const TemplateURL* template_url = &table_model_->GetTemplateURL(last_row);
+      last_view_row = table_view_->model_to_view(*i);
+      const TemplateURL* template_url = &table_model_->GetTemplateURL(*i);
       // Make sure to remove from the table model first, otherwise the
       // TemplateURL would be freed.
-      table_model_->Remove(last_row);
+      table_model_->Remove(*i);
       url_model_->Remove(template_url);
     }
-    if (last_row >= table_model_->RowCount())
-      last_row = table_model_->RowCount() - 1;
-    if (last_row >= 0)
-      table_view_->Select(last_row);
+    if (last_view_row >= table_model_->RowCount())
+      last_view_row = table_model_->RowCount() - 1;
+    if (last_view_row >= 0)
+      table_view_->Select(table_view_->view_to_model(last_view_row));
     url_model_->AddObserver(this);
 
     // We may have removed the default provider.  Enable the Suggest checkbox
@@ -676,5 +684,5 @@ void KeywordEditorView::MakeDefaultSearchProvider(int index) {
   table_model_->MoveToMainGroup(index);
 
   // And select it.
-  table_view_->Select(new_index);
+  table_view_->Select(table_model_->IndexOfTemplateURL(keyword));
 }
diff --git a/chrome/test/unit/unittests.vcproj b/chrome/test/unit/unittests.vcproj
index 68950ad..64e7d57 100644
--- a/chrome/test/unit/unittests.vcproj
+++ b/chrome/test/unit/unittests.vcproj
@@ -407,6 +407,14 @@
 			</File>
 		</Filter>
 		<Filter
+			Name="TestTableView"
+			>
+			<File
+				RelativePath="..\..\views\table_view_unittest.cc"
+				>
+			</File>
+		</Filter>
+		<Filter
 			Name="TestAnimation"
 			>
 			<File
diff --git a/chrome/views/group_table_view.cc b/chrome/views/group_table_view.cc
index 5088997..4642770 100644
--- a/chrome/views/group_table_view.cc
+++ b/chrome/views/group_table_view.cc
@@ -124,7 +124,31 @@ void GroupTableView::OnKeyDown(unsigned short virtual_keycode) {
   }
 }
 
-void GroupTableView::OnSelectedStateChanged(int item, bool is_selected) {
+void GroupTableView::PrepareForSort() {
+  GroupRange range;
+  int row_count = RowCount();
+  model_index_to_range_start_map_.clear();
+  for (int model_row = 0; model_row < row_count;) {
+    model_->GetGroupRangeForItem(model_row, &range);
+    for (int range_counter = 0; range_counter < range.length; range_counter++)
+      model_index_to_range_start_map_[range_counter + model_row] = model_row;
+    model_row += range.length;
+  }
+}
+
+int GroupTableView::CompareRows(int model_row1, int model_row2) {
+  int range1 = model_index_to_range_start_map_[model_row1];
+  int range2 = model_index_to_range_start_map_[model_row2];
+  if (range1 == range2) {
+    // The two rows are in the same group, sort so that items in the same group
+    // always appear in the same order.
+    return model_row1 - model_row2;
+  }
+  // Sort by the first entry of each of the groups.
+  return TableView::CompareRows(range1, range2);
+}
+
+void GroupTableView::OnSelectedStateChanged(int model_row, bool is_selected) {
   // The goal is to make sure all items for a same group are in a consistent
   // state in term of selection. When a user clicks an item, several selection
   // messages are sent, possibly including unselecting all currently selected
@@ -136,14 +160,14 @@ void GroupTableView::OnSelectedStateChanged(int item, bool is_selected) {
         sync_selection_factory_.NewRunnableMethod(
             &GroupTableView::SyncSelection));
   }
-  TableView::OnSelectedStateChanged(item, is_selected);
+  TableView::OnSelectedStateChanged(model_row, is_selected);
 }
 
 // Draws the line separator betweens the groups.
-void GroupTableView::PostPaint(int row, int column, bool selected,
+void GroupTableView::PostPaint(int model_row, int column, bool selected,
                                const CRect& bounds, HDC hdc) {
   GroupRange group_range;
-  model_->GetGroupRangeForItem(row, &group_range);
+  model_->GetGroupRangeForItem(model_row, &group_range);
 
   // We always paint a vertical line at the end of the last cell.
   HPEN hPen = CreatePen(PS_SOLID, kSeparatorLineThickness, kSeparatorLineColor);
@@ -153,7 +177,7 @@ void GroupTableView::PostPaint(int row, int column, bool selected,
   LineTo(hdc, x, bounds.bottom);
 
   // We paint a separator line after the last item of a group.
-  if (row == (group_range.start + group_range.length - 1)) {
+  if (model_row == (group_range.start + group_range.length - 1)) {
     int y = static_cast<int>(bounds.bottom - kSeparatorLineThickness);
     MoveToEx(hdc, 0, y, NULL);
     LineTo(hdc, bounds.Width(), y);
@@ -165,5 +189,5 @@ void GroupTableView::PostPaint(int row, int column, bool selected,
 std::string GroupTableView::GetClassName() const {
   return kViewClassName;
 }
-}  // Namespace
 
+}  // Namespace
diff --git a/chrome/views/group_table_view.h b/chrome/views/group_table_view.h
index 6772542..d534132 100644
--- a/chrome/views/group_table_view.h
+++ b/chrome/views/group_table_view.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_VIEWS_GROUP_TABLE_VIEW_H__
-#define CHROME_VIEWS_GROUP_TABLE_VIEW_H__
+#ifndef CHROME_VIEWS_GROUP_TABLE_VIEW_H_
+#define CHROME_VIEWS_GROUP_TABLE_VIEW_H_
 
 #include "base/task.h"
 #include "chrome/views/table_view.h"
@@ -43,11 +43,11 @@ class GroupTableView : public TableView {
  protected:
   // Notification from the ListView that the selected state of an item has
   // changed.
-  void OnSelectedStateChanged(int item, bool is_selected);
+  void OnSelectedStateChanged(int model_row, bool is_selected);
 
   // Extra-painting required to draw the separator line between groups.
   virtual bool ImplementPostPaint() { return true; }
-  virtual void PostPaint(int row, int column, bool selected,
+  virtual void PostPaint(int model_row, int column, bool selected,
                          const CRect& bounds, HDC device_context);
 
   // In order to make keyboard navigation possible (using the Up and Down
@@ -56,19 +56,27 @@ class GroupTableView : public TableView {
   // needs to be set on a group item when a group is selected.
   virtual void OnKeyDown(unsigned short virtual_keycode);
 
+  // Overriden to make sure rows in the same group stay grouped together.
+  virtual int CompareRows(int model_row1, int model_row2);
+
+  // Updates model_index_to_range_start_map_ from the model.
+  virtual void PrepareForSort();
+
  private:
   // Make the selection of group consistent.
   void SyncSelection();
 
-  GroupTableModel *model_;
+  GroupTableModel* model_;
 
   // A factory to make the selection consistent among groups.
   ScopedRunnableMethodFactory<GroupTableView> sync_selection_factory_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(GroupTableView);
+  // Maps from model row to start of group.
+  std::map<int,int> model_index_to_range_start_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(GroupTableView);
 };
 
 }  // namespace ChromeViews
 
-#endif  // CHROME_VIEWS_GROUP_TABLE_VIEW_H__
-
+#endif  // CHROME_VIEWS_GROUP_TABLE_VIEW_H_
diff --git a/chrome/views/table_view.cc b/chrome/views/table_view.cc
index ab4d717..e50cf04 100644
--- a/chrome/views/table_view.cc
+++ b/chrome/views/table_view.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <algorithm>
 #include <windowsx.h>
 
 #include "chrome/views/table_view.h"
@@ -11,12 +12,15 @@
 #include "base/gfx/skia_utils.h"
 #include "chrome/common/gfx/chrome_canvas.h"
 #include "chrome/common/gfx/favicon_size.h"
+#include "chrome/common/gfx/icon_util.h"
 #include "chrome/common/resource_bundle.h"
 #include "chrome/common/win_util.h"
 #include "chrome/views/hwnd_view.h"
 #include "chrome/views/view_container.h"
 #include "SkBitmap.h"
 #include "SkColorFilter.h"
+#include "unicode/coll.h"
+#include "unicode/uchar.h"
 
 namespace ChromeViews {
 
@@ -25,10 +29,49 @@ const int kListViewTextPadding = 15;
 // Additional column width necessary if column has icons.
 const int kListViewIconWidthAndPadding = 18;
 
+const int kImageSize = 18;
+
+// TableModel -----------------------------------------------------------------
+
+// Used for sorting.
+static Collator* collator = NULL;
+
 SkBitmap TableModel::GetIcon(int row) {
   return SkBitmap();
 }
 
+int TableModel::CompareValues(int row1, int row2, int column_id) {
+  DCHECK(row1 >= 0 && row1 < RowCount() &&
+         row2 >= 0 && row2 < RowCount());
+  std::wstring value1 = GetText(row1, column_id);
+  std::wstring value2 = GetText(row2, column_id);
+
+  if (!collator) {
+    UErrorCode create_status = U_ZERO_ERROR;
+    collator = Collator::createInstance(create_status);
+    if (!U_SUCCESS(create_status)) {
+      collator = NULL;
+      NOTREACHED();
+    }
+  }
+  
+  if (collator) {
+    UErrorCode compare_status = U_ZERO_ERROR;
+    UCollationResult compare_result = collator->compare(
+        static_cast<const UChar*>(value1.c_str()),
+        static_cast<int>(value1.length()),
+        static_cast<const UChar*>(value2.c_str()),
+        static_cast<int>(value2.length()),
+        compare_status);
+    DCHECK(U_SUCCESS(compare_status));
+    return compare_result;
+  }
+  NOTREACHED();
+  return 0;
+}
+
+// TableView ------------------------------------------------------------------
+
 TableView::TableView(TableModel* model,
                      const std::vector<TableColumn>& columns,
                      TableTypes table_type,
@@ -40,7 +83,6 @@ TableView::TableView(TableModel* model,
       visible_columns_(),
       all_columns_(),
       column_count_(static_cast<int>(columns.size())),
-      cache_data_(true),
       table_type_(table_type),
       single_selection_(single_selection),
       ignore_listview_change_(false),
@@ -77,6 +119,31 @@ void TableView::SetModel(TableModel* model) {
     OnModelChanged();
 }
 
+void TableView::SetSortDescriptors(const SortDescriptors& sort_descriptors) {
+  if (!sort_descriptors_.empty()) {
+    ResetColumnSortImage(sort_descriptors_[0].column_id,
+                         NO_SORT);
+  }
+  sort_descriptors_ = sort_descriptors;
+  if (!sort_descriptors_.empty()) {
+    ResetColumnSortImage(
+        sort_descriptors_[0].column_id,
+        sort_descriptors_[0].ascending ? ASCENDING_SORT : DESCENDING_SORT);
+  }
+  if (!list_view_)
+    return;
+
+  // For some reason we have to turn off/on redraw, otherwise the display
+  // isn't updated when done.
+  SendMessage(list_view_, WM_SETREDRAW, static_cast<WPARAM>(FALSE), 0);
+
+  UpdateItemsLParams(0, 0);
+
+  SortItemsAndUpdateMapping();
+
+  SendMessage(list_view_, WM_SETREDRAW, static_cast<WPARAM>(TRUE), 0);
+}
+
 void TableView::DidChangeBounds(const CRect& previous,
                                 const CRect& current) {
   if (!list_view_)
@@ -103,11 +170,11 @@ int TableView::SelectedRowCount() {
   return ListView_GetSelectedCount(list_view_);
 }
 
-void TableView::Select(int item) {
+void TableView::Select(int model_row) {
   if (!list_view_)
     return;
 
-  DCHECK(item >= 0 && item < RowCount());
+  DCHECK(model_row >= 0 && model_row < RowCount());
   SendMessage(list_view_, WM_SETREDRAW, static_cast<WPARAM>(FALSE), 0);
   ignore_listview_change_ = true;
 
@@ -115,41 +182,44 @@ void TableView::Select(int item) {
   ListView_SetItemState(list_view_, -1, 0, LVIS_SELECTED);
 
   // Select the specified item.
-  ListView_SetItemState(list_view_, item, LVIS_SELECTED | LVIS_FOCUSED,
+  int view_row = model_to_view(model_row);
+  ListView_SetItemState(list_view_, view_row, LVIS_SELECTED | LVIS_FOCUSED,
                         LVIS_SELECTED | LVIS_FOCUSED);
 
   // Make it visible.
-  ListView_EnsureVisible(list_view_, item, FALSE);
+  ListView_EnsureVisible(list_view_, view_row, FALSE);
   ignore_listview_change_ = false;
   SendMessage(list_view_, WM_SETREDRAW, static_cast<WPARAM>(TRUE), 0);
   if (table_view_observer_)
     table_view_observer_->OnSelectionChanged();
 }
 
-void TableView::SetSelectedState(int item, bool state) {
+void TableView::SetSelectedState(int model_row, bool state) {
   if (!list_view_)
     return;
 
-  DCHECK(item >= 0 && item < RowCount());
+  DCHECK(model_row >= 0 && model_row < RowCount());
 
   ignore_listview_change_ = true;
 
   // Select the specified item.
-  ListView_SetItemState(list_view_, item, LVIS_SELECTED, LVIS_SELECTED);
+  ListView_SetItemState(list_view_, model_to_view(model_row),
+                        state ? LVIS_SELECTED : 0,  LVIS_SELECTED);
 
   ignore_listview_change_ = false;
 }
 
-void TableView::SetFocusOnItem(int item) {
+void TableView::SetFocusOnItem(int model_row) {
   if (!list_view_)
     return;
 
-  DCHECK(item >= 0 && item < RowCount());
+  DCHECK(model_row >= 0 && model_row < RowCount());
 
   ignore_listview_change_ = true;
 
   // Set the focus to the given item.
-  ListView_SetItemState(list_view_, item, LVIS_FOCUSED, LVIS_FOCUSED);
+  ListView_SetItemState(list_view_, model_to_view(model_row), LVIS_FOCUSED,
+                        LVIS_FOCUSED);
 
   ignore_listview_change_ = false;
 }
@@ -158,30 +228,30 @@ int TableView::FirstSelectedRow() {
   if (!list_view_)
     return -1;
 
-  return ListView_GetNextItem(list_view_, -1, LVNI_ALL | LVIS_SELECTED);
+  int view_row = ListView_GetNextItem(list_view_, -1, LVNI_ALL | LVIS_SELECTED);
+  return view_row == -1 ? -1 : view_to_model(view_row);
 }
 
-
-bool TableView::IsItemSelected(int item) {
+bool TableView::IsItemSelected(int model_row) {
   if (!list_view_)
     return false;
 
-  DCHECK(item >= 0 && item < RowCount());
-  return (ListView_GetItemState(list_view_, item, LVIS_SELECTED) ==
-              LVIS_SELECTED);
+  DCHECK(model_row >= 0 && model_row < RowCount());
+  return (ListView_GetItemState(list_view_, model_to_view(model_row),
+                                LVIS_SELECTED) == LVIS_SELECTED);
 }
 
-bool TableView::ItemHasTheFocus(int item) {
+bool TableView::ItemHasTheFocus(int model_row) {
   if (!list_view_)
     return false;
 
-  DCHECK(item >= 0 && item < RowCount());
-  return (ListView_GetItemState(list_view_, item, LVIS_FOCUSED) ==
-              LVIS_FOCUSED);
+  DCHECK(model_row >= 0 && model_row < RowCount());
+  return (ListView_GetItemState(list_view_, model_to_view(model_row),
+                                LVIS_FOCUSED) == LVIS_FOCUSED);
 }
 
 TableView::iterator TableView::SelectionBegin() {
-  return TableView::iterator(this, LastSelectedIndex());
+  return TableView::iterator(this, LastSelectedViewIndex());
 }
 
 TableView::iterator TableView::SelectionEnd() {
@@ -194,11 +264,7 @@ void TableView::OnItemsChanged(int start, int length) {
 
   if (length == -1) {
     DCHECK(start >= 0);
-    if (cache_data_) {
-      length = model_->RowCount() - start;
-    } else {
-      length = RowCount() - start;
-    }
+    length = model_->RowCount() - start;
   }
   int row_count = RowCount();
   DCHECK(start >= 0 && length > 0 && start + length <= row_count);
@@ -214,7 +280,7 @@ void TableView::OnItemsChanged(int start, int length) {
     lv_item.mask = LVIF_IMAGE;
     for (int i = start; i < start + length; ++i) {
       // Retrieve the current icon index.
-      lv_item.iItem = i;
+      lv_item.iItem = model_to_view(i);
       BOOL r = ListView_GetItem(list_view_, &lv_item);
       DCHECK(r);
       // Set the current icon index to the other image.
@@ -224,15 +290,10 @@ void TableView::OnItemsChanged(int start, int length) {
       DCHECK(r);
     }
   }
-  if (!cache_data_) {
-    ListView_RedrawItems(list_view_, start, start + length);
-  } else {
-    UpdateListViewCache(start, length, false);
-  }
+  UpdateListViewCache(start, length, false);
   SendMessage(list_view_, WM_SETREDRAW, static_cast<WPARAM>(TRUE), 0);
 }
 
-
 void TableView::OnModelChanged() {
   if (!list_view_)
     return;
@@ -250,12 +311,7 @@ void TableView::OnItemsAdded(int start, int length) {
 
   DCHECK(start >= 0 && length > 0 && start <= RowCount());
   SendMessage(list_view_, WM_SETREDRAW, static_cast<WPARAM>(FALSE), 0);
-  if (!cache_data_) {
-    ListView_SetItemCount(list_view_, model_->RowCount());
-    ListView_RedrawItems(list_view_, start, start + length);
-  } else {
-    UpdateListViewCache(start, length, true);
-  }
+  UpdateListViewCache(start, length, true);
   SendMessage(list_view_, WM_SETREDRAW, static_cast<WPARAM>(TRUE), 0);
 }
 
@@ -263,23 +319,62 @@ void TableView::OnItemsRemoved(int start, int length) {
   if (!list_view_)
     return;
 
-  DCHECK(start >= 0 && length > 0 && start + length <= RowCount());
+  if (start < 0 || length < 0 || start + length > RowCount()) {
+    NOTREACHED();
+    return;
+  }
+
   SendMessage(list_view_, WM_SETREDRAW, static_cast<WPARAM>(FALSE), 0);
+
   bool had_selection = (SelectedRowCount() > 0);
-  if (!cache_data_) {
-    // TODO(sky): Make sure this triggers a repaint.
-    ListView_SetItemCount(list_view_, model_->RowCount());
+  int old_row_count = RowCount();
+  if (start == 0 && length == RowCount()) {
+    // Everything was removed.
+    ListView_DeleteAllItems(list_view_);
+    view_to_model_.reset(NULL);
+    model_to_view_.reset(NULL);
   } else {
-    // Update the cache.
-    if (start == 0 && length == RowCount()) {
-      ListView_DeleteAllItems(list_view_);
+    // Only a portion of the data was removed.
+    if (is_sorted()) {
+      int new_row_count = model_->RowCount();
+      std::vector<int> view_items_to_remove;
+      view_items_to_remove.reserve(length);
+      // Iterate through the elements, updating the view_to_model_ mapping
+      // as well as collecting the rows that need to be deleted.
+      for (int i = 0, removed_count = 0; i < old_row_count; ++i) {
+        int model_index = view_to_model(i);
+        if (model_index >= start) {
+          if (model_index < start + length) {
+            // This item was removed.
+            view_items_to_remove.push_back(i);
+            model_index = -1;
+          } else {
+            model_index -= length;
+          }
+        }
+        if (model_index >= 0) {
+          view_to_model_[i - static_cast<int>(view_items_to_remove.size())] =
+              model_index;
+        }
+      }
+
+      // Update the model_to_view mapping from the updated view_to_model
+      // mapping.
+      for (int i = 0; i < new_row_count; ++i)
+        model_to_view_[view_to_model_[i]] = i;
+
+      // And finally delete the items. We do this backwards as the items were
+      // added ordered smallest to largest.
+      for (int i = length - 1; i >= 0; --i)
+        ListView_DeleteItem(list_view_, view_items_to_remove[i]);
     } else {
-      for (int i = 0; i < length; ++i) {
+      for (int i = 0; i < length; ++i)
         ListView_DeleteItem(list_view_, start);
-      }
     }
   }
+
   SendMessage(list_view_, WM_SETREDRAW, static_cast<WPARAM>(TRUE), 0);
+
   // We don't seem to get notification in this case.
   if (table_view_observer_ && had_selection && RowCount() == 0)
     table_view_observer_->OnSelectionChanged();
@@ -296,6 +391,16 @@ void TableView::SetColumns(const std::vector<TableColumn>& columns) {
        i != columns.end(); ++i) {
     AddColumn(*i);
   }
+
+  // Remove any sort descriptors that are no longer valid.
+  SortDescriptors sort = sort_descriptors();
+  for (SortDescriptors::iterator i = sort.begin(); i != sort.end();) {
+    if (all_columns_.count(i->column_id) == 0)
+      i = sort.erase(i);
+    else
+      ++i;
+  }
+  sort_descriptors_ = sort;
 }
 
 void TableView::OnColumnsChanged() {
@@ -385,7 +490,7 @@ void TableView::SetCustomColorsEnabled(bool custom_colors_enabled) {
   custom_colors_enabled_ = custom_colors_enabled;
 }
 
-bool TableView::GetCellColors(int row,
+bool TableView::GetCellColors(int model_row,
                               int column,
                               ItemColor* foreground,
                               ItemColor* background,
@@ -444,8 +549,6 @@ HWND TableView::CreateNativeControl(HWND parent_container) {
   int style = WS_CHILD | LVS_REPORT | LVS_SHOWSELALWAYS;
   if (single_selection_)
     style |= LVS_SINGLESEL;
-  if (!cache_data_)
-    style |= LVS_OWNERDATA;
   // If there's only one column and the title string is empty, don't show a
   // header.
   if (all_columns_.size() == 1) {
@@ -497,28 +600,22 @@ HWND TableView::CreateNativeControl(HWND parent_container) {
   }
 
   // Set the # of rows.
-  if (cache_data_) {
-    UpdateListViewCache(0, model_->RowCount(), true);
-  } else if (table_type_ == CHECK_BOX_AND_TEXT) {
-    ListView_SetItemCount(list_view_, model_->RowCount());
-    ListView_SetCallbackMask(list_view_, LVIS_STATEIMAGEMASK);
-  }
+  UpdateListViewCache(0, model_->RowCount(), true);
 
-  // Load the default icon.
   if (table_type_ == ICON_AND_TEXT) {
-    HIMAGELIST image_list = ImageList_Create(18, 18, ILC_COLOR, 1, 1);
-    // TODO(jcampan): include a default icon image.
-    // This icon will not be found. The list view will still layout with the
-    // right space for the icon so we can paint our own icon.
-    HBITMAP image = LoadBitmap(NULL, L"IDR_WHATEVER");
-    ImageList_Add(image_list, image, NULL);
-    DeleteObject(image);
+    HIMAGELIST image_list =
+        ImageList_Create(kImageSize, kImageSize, ILC_COLOR32, 2, 2);
     // We create 2 phony images because we are going to switch images at every
     // refresh in order to force a refresh of the icon area (somehow the clip
     // rect does not include the icon).
-    image = LoadBitmap(NULL, L"IDR_WHATEVER_AGAIN");
-    ImageList_Add(image_list, image, NULL);
-    DeleteObject(image);
+    ChromeCanvas canvas(kImageSize, kImageSize, false);
+    // Make the background completely transparent.
+    canvas.drawColor(SK_ColorBLACK, SkPorterDuff::kClear_Mode);
+    HICON empty_icon =
+        IconUtil::CreateHICONFromSkBitmap(canvas.ExtractBitmap());
+    ImageList_AddIcon(image_list, empty_icon);
+    ImageList_AddIcon(image_list, empty_icon);
+    DeleteObject(empty_icon);
     ListView_SetImageList(list_view_, image_list, LVSIL_SMALL);
   }
 
@@ -548,6 +645,114 @@ HWND TableView::CreateNativeControl(HWND parent_container) {
   return list_view_;
 }
 
+void TableView::ToggleSortOrder(int column_id) {
+  SortDescriptors sort = sort_descriptors();
+  if (!sort.empty() && sort[0].column_id == column_id) {
+    sort[0].ascending = !sort[0].ascending;
+  } else {
+    SortDescriptor descriptor(column_id, true);
+    sort.insert(sort.begin(), descriptor);
+    if (sort.size() > 2) {
+      // Only persist two sort descriptors.
+      sort.resize(2);
+    }
+  }
+  SetSortDescriptors(sort);
+}
+
+void TableView::UpdateItemsLParams(int start, int length) {
+  LVITEM item;
+  memset(&item, 0, sizeof(LVITEM));
+  item.mask = LVIF_PARAM;
+  int row_count = RowCount();
+  for (int i = 0; i < row_count; ++i) {
+    item.iItem = i;
+    int model_index = view_to_model(i);
+    if (length > 0 && model_index >= start)
+      model_index += length;
+    item.lParam = static_cast<LPARAM>(model_index);
+    ListView_SetItem(list_view_, &item);
+  }
+}
+
+void TableView::SortItemsAndUpdateMapping() {
+  if (!is_sorted()) {
+    ListView_SortItems(list_view_, &TableView::NaturalSortFunc, this);
+    view_to_model_.reset(NULL);
+    model_to_view_.reset(NULL);
+    return;
+  }
+
+  PrepareForSort();
+
+  // Sort the items.
+  ListView_SortItems(list_view_, &TableView::SortFunc, this);
+
+  // Cleanup the collator.
+  if (collator) {
+    delete collator;
+    collator = NULL;
+  }
+
+  // Update internal mapping to match how items were actually sorted.
+  int row_count = RowCount();
+  model_to_view_.reset(new int[row_count]);
+  view_to_model_.reset(new int[row_count]);
+  LVITEM item;
+  memset(&item, 0, sizeof(LVITEM));
+  item.mask = LVIF_PARAM;
+  for (int i = 0; i < row_count; ++i) {
+    item.iItem = i;
+    ListView_GetItem(list_view_, &item);
+    int model_index = static_cast<int>(item.lParam);
+    view_to_model_[i] = model_index;
+    model_to_view_[model_index] = i;
+  }
+}
+
+// static
+int CALLBACK TableView::SortFunc(LPARAM model_index_1_p,
+                                 LPARAM model_index_2_p,
+                                 LPARAM table_view_param) {
+  int model_index_1 = static_cast<int>(model_index_1_p);
+  int model_index_2 = static_cast<int>(model_index_2_p);
+  TableView* table_view = reinterpret_cast<TableView*>(table_view_param);
+  return table_view->CompareRows(model_index_1, model_index_2);
+}
+
+// static
+int CALLBACK TableView::NaturalSortFunc(LPARAM model_index_1_p,
+                                        LPARAM model_index_2_p,
+                                        LPARAM table_view_param) {
+  return model_index_1_p - model_index_2_p;
+}
+
+void TableView::ResetColumnSortImage(int column_id, SortDirection direction) {
+  if (!list_view_ || column_id == -1)
+    return;
+
+  std::vector<int>::const_iterator i =
+      std::find(visible_columns_.begin(), visible_columns_.end(), column_id);
+  if (i == visible_columns_.end())
+    return;
+
+  HWND header = ListView_GetHeader(list_view_);
+  if (!header)
+    return;
+
+  int column_index = static_cast<int>(i - visible_columns_.begin());
+  HDITEM header_item;
+  memset(&header_item, 0, sizeof(header_item));
+  header_item.mask = HDI_FORMAT;
+  Header_GetItem(header, column_index, &header_item);
+  header_item.fmt &= ~(HDF_SORTUP | HDF_SORTDOWN);
+  if (direction == ASCENDING_SORT)
+    header_item.fmt |= HDF_SORTUP;
+  else if (direction == DESCENDING_SORT)
+    header_item.fmt |= HDF_SORTDOWN;
+  Header_SetItem(header, column_index, &header_item);
+}
+
 void TableView::InsertColumn(const TableColumn& tc, int index) {
   if (!list_view_)
     return;
@@ -577,6 +782,11 @@ void TableView::InsertColumn(const TableColumn& tc, int index) {
   column.iSubItem = index + 1;
   SendMessage(list_view_, LVM_INSERTCOLUMN, index,
               reinterpret_cast<LPARAM>(&column));
+  if (is_sorted() && sort_descriptors_[0].column_id == tc.id) {
+    ResetColumnSortImage(
+        tc.id,
+        sort_descriptors_[0].ascending ? ASCENDING_SORT : DESCENDING_SORT);
+  }
 }
 
 LRESULT TableView::OnNotify(int w_param, NMHDR* hdr) {
@@ -585,6 +795,7 @@ LRESULT TableView::OnNotify(int w_param, NMHDR* hdr) {
       // Draw notification. dwDragState indicates the current stage of drawing.
       return OnCustomDraw(reinterpret_cast<NMLVCUSTOMDRAW*>(hdr));
     }
+
     case LVN_ITEMCHANGED: {
       // Notification that the state of an item has changed. The state
       // includes such things as whether the item is selected or checked.
@@ -594,7 +805,8 @@ LRESULT TableView::OnNotify(int w_param, NMHDR* hdr) {
             (state_change->uNewState & LVIS_SELECTED)) {
           // Selected state of the item changed.
           bool is_selected = ((state_change->uNewState & LVIS_SELECTED) != 0);
-          OnSelectedStateChanged(state_change->iItem, is_selected);
+          OnSelectedStateChanged(view_to_model(state_change->iItem),
+                                 is_selected);
         }
         if ((state_change->uOldState & LVIS_STATEIMAGEMASK) !=
             (state_change->uNewState & LVIS_STATEIMAGEMASK)) {
@@ -602,17 +814,20 @@ LRESULT TableView::OnNotify(int w_param, NMHDR* hdr) {
           bool is_checked =
             ((state_change->uNewState & LVIS_STATEIMAGEMASK) ==
              INDEXTOSTATEIMAGEMASK(2));
-          OnCheckedStateChanged(state_change->iItem, is_checked);
+          OnCheckedStateChanged(view_to_model(state_change->iItem),
+                                is_checked);
         }
       }
       break;
     }
+
     case HDN_BEGINTRACKW:
     case HDN_BEGINTRACKA:
       // Prevent clicks so columns cannot be resized.
       if (!resizable_columns_)
         return TRUE;
       break;
+
     case NM_DBLCLK:
       OnDoubleClick();
       break;
@@ -626,6 +841,14 @@ LRESULT TableView::OnNotify(int w_param, NMHDR* hdr) {
       break;
     }
 
+    case LVN_COLUMNCLICK: {
+      const TableColumn& column = GetColumnAtPosition(
+          reinterpret_cast<NMLISTVIEW*>(hdr)->iSubItem);
+      if (column.sortable)
+        ToggleSortOrder(column.id);
+      break;
+    }
+
     default:
       break;
   }
@@ -635,13 +858,44 @@ LRESULT TableView::OnNotify(int w_param, NMHDR* hdr) {
 void TableView::OnDestroy() {
   if (table_type_ == ICON_AND_TEXT) {
     HIMAGELIST image_list =
-      ListView_GetImageList(GetNativeControlHWND(), LVSIL_SMALL);
+        ListView_GetImageList(GetNativeControlHWND(), LVSIL_SMALL);
     DCHECK(image_list);
     if (image_list)
       ImageList_Destroy(image_list);
   }
 }
 
+// Returns result, unless ascending is false in which case -result is returned.
+static int SwapCompareResult(int result, bool ascending) {
+  return ascending ? result : -result;
+}
+
+int TableView::CompareRows(int model_row1, int model_row2) {
+  if (model_->HasGroups()) {
+    // By default ListView sorts the elements regardless of groups. In such
+    // a situation the groups display only the items they contain. This results
+    // in the visual order differing from the item indices. I could not find
+    // a way to iterate over the visual order in this situation. As a workaround
+    // this forces the items to be sorted by groups as well, which means the
+    // visual order matches the item indices.
+    int g1 = model_->GetGroupID(model_row1);
+    int g2 = model_->GetGroupID(model_row2);
+    if (g1 != g2)
+      return g1 - g2;
+  }
+  int sort_result = model_->CompareValues(
+      model_row1, model_row2, sort_descriptors_[0].column_id);
+  if (sort_result == 0 && sort_descriptors_.size() > 1 &&
+      sort_descriptors_[1].column_id != -1) {
+    // Try the secondary sort.
+    return SwapCompareResult(
+        model_->CompareValues(model_row1, model_row2,
+                              sort_descriptors_[1].column_id),
+        sort_descriptors_[1].ascending);
+  }
+  return SwapCompareResult(sort_result, sort_descriptors_[0].ascending);
+}
+
 LRESULT TableView::OnCustomDraw(NMLVCUSTOMDRAW* draw_info) {
   switch (draw_info->nmcd.dwDrawStage) {
     case CDDS_PREPAINT: {
@@ -668,7 +922,8 @@ LRESULT TableView::OnCustomDraw(NMLVCUSTOMDRAW* draw_info) {
         LOGFONT logfont;
         GetObject(GetWindowFont(list_view_), sizeof(logfont), &logfont);
 
-        if (GetCellColors(static_cast<int>(draw_info->nmcd.dwItemSpec),
+        if (GetCellColors(view_to_model(
+                              static_cast<int>(draw_info->nmcd.dwItemSpec)),
                           draw_info->iSubItem,
                           &foreground,
                           &background,
@@ -693,19 +948,19 @@ LRESULT TableView::OnCustomDraw(NMLVCUSTOMDRAW* draw_info) {
     }
     case CDDS_ITEMPOSTPAINT: {
       DCHECK((table_type_ == ICON_AND_TEXT) || (ImplementPostPaint()));
-      int n_item = static_cast<int>(draw_info->nmcd.dwItemSpec);
+      int view_index = static_cast<int>(draw_info->nmcd.dwItemSpec);
       // We get notifications for empty items, just ignore them.
-      if (n_item >= model_->RowCount()) {
+      if (view_index >= model_->RowCount())
         return CDRF_DODEFAULT;
-      }
+      int model_index = view_to_model(view_index);
       LRESULT r = CDRF_DODEFAULT;
       // First let's take care of painting the right icon.
       if (table_type_ == ICON_AND_TEXT) {
-        SkBitmap image = model_->GetIcon(n_item);
+        SkBitmap image = model_->GetIcon(model_index);
         if (!image.isNull()) {
           // Get the rect that holds the icon.
           CRect icon_rect, client_rect;
-          if (ListView_GetItemRect(list_view_, n_item, &icon_rect,
+          if (ListView_GetItemRect(list_view_, view_index, &icon_rect,
                                    LVIR_ICON) &&
               GetClientRect(list_view_, &client_rect)) {
             CRect intersection;
@@ -718,7 +973,7 @@ LRESULT TableView::OnCustomDraw(NMLVCUSTOMDRAW* draw_info) {
 
               // It seems the state in nmcd.uItemState is not correct.
               // We'll retrieve it explicitly.
-              int selected = ListView_GetItemState(list_view_, n_item,
+              int selected = ListView_GetItemState(list_view_, view_index,
                                                    LVIS_SELECTED);
               int bg_color_index;
               if (!IsEnabled())
@@ -758,8 +1013,9 @@ LRESULT TableView::OnCustomDraw(NMLVCUSTOMDRAW* draw_info) {
       }
       if (ImplementPostPaint()) {
         CRect cell_rect;
-        if (ListView_GetItemRect(list_view_, n_item, &cell_rect, LVIR_BOUNDS)) {
-          PostPaint(n_item, 0, false, cell_rect, draw_info->nmcd.hdc);
+        if (ListView_GetItemRect(list_view_, view_index, &cell_rect,
+                                 LVIR_BOUNDS)) {
+          PostPaint(model_index, 0, false, cell_rect, draw_info->nmcd.hdc);
           r = CDRF_SKIPDEFAULT;
         }
       }
@@ -847,21 +1103,29 @@ void TableView::GetPreferredSize(CSize* out) {
   *out = preferred_size_;
 }
 
-
 void TableView::UpdateListViewCache0(int start, int length, bool add) {
+  if (is_sorted()) {
+    if (add)
+      UpdateItemsLParams(start, length);
+    else
+      UpdateItemsLParams(0, 0);
+  }
+
   LVITEM item = {0};
   int start_column = 0;
   int max_row = start + length;
   const bool has_groups =
       (win_util::GetWinVersion() > win_util::WINVERSION_2000 &&
        model_->HasGroups());
-  if (has_groups)
-    item.mask = LVIF_GROUPID;
   if (add) {
+    if (has_groups)
+      item.mask = LVIF_GROUPID;
+    item.mask |= LVIF_PARAM;
     for (int i = start; i < max_row; ++i) {
       item.iItem = i;
       if (has_groups)
         item.iGroupId = model_->GetGroupID(i);
+      item.lParam = i;
       ListView_InsertItem(list_view_, &item);
     }
   }
@@ -878,14 +1142,12 @@ void TableView::UpdateListViewCache0(int start, int length, bool add) {
     item.stateMask = LVIS_STATEIMAGEMASK;
     for (int i = start; i < max_row; ++i) {
       std::wstring text = model_->GetText(i, visible_columns_[0]);
-      item.iItem = i;
+      item.iItem = add ? i : model_to_view(i);
       item.pszText = const_cast<LPWSTR>(text.c_str());
       item.state = INDEXTOSTATEIMAGEMASK(model_->IsChecked(i) ? 2 : 1);
       ListView_SetItem(list_view_, &item);
     }
   }
-  if (start_column == column_count_)
-    return;
 
   item.stateMask = 0;
   item.mask = LVIF_TEXT;
@@ -896,7 +1158,7 @@ void TableView::UpdateListViewCache0(int start, int length, bool add) {
     TableColumn& col = all_columns_[visible_columns_[j]];
     int max_text_width = ListView_GetStringWidth(list_view_, col.title.c_str());
     for (int i = start; i < max_row; ++i) {
-      item.iItem = i;
+      item.iItem = add ? i : model_to_view(i);
       item.iSubItem = j;
       std::wstring text = model_->GetText(i, visible_columns_[j]);
       item.pszText = const_cast<LPWSTR>(text.c_str());
@@ -917,10 +1179,18 @@ void TableView::UpdateListViewCache0(int start, int length, bool add) {
 
     // Protect against partial update.
     if (max_text_width > col.min_visible_width ||
-      (start == 0 && length == model_->RowCount())) {
+        (start == 0 && length == model_->RowCount())) {
       col.min_visible_width = max_text_width;
     }
   }
+
+  if (is_sorted()) {
+    // NOTE: As most of our tables are smallish I'm not going to optimize this.
+    // If our tables become large and frequently update, then it'll make sense
+    // to optimize this.
+
+    SortItemsAndUpdateMapping();
+  }
 }
 
 void TableView::OnDoubleClick() {
@@ -941,15 +1211,14 @@ void TableView::OnKeyDown(unsigned short virtual_keycode) {
   }
 }
 
-void TableView::OnCheckedStateChanged(int item, bool is_checked) {
-  if (!ignore_listview_change_) {
-    model_->SetChecked(item, is_checked);
-  }
+void TableView::OnCheckedStateChanged(int model_row, bool is_checked) {
+  if (!ignore_listview_change_)
+    model_->SetChecked(model_row, is_checked);
 }
 
-int TableView::PreviousSelectedIndex(int item) {
-  DCHECK(item >= 0);
-  if (!list_view_ || item <= 0)
+int TableView::PreviousSelectedViewIndex(int view_index) {
+  DCHECK(view_index >= 0);
+  if (!list_view_ || view_index <= 0)
     return -1;
 
   int row_count = RowCount();
@@ -959,13 +1228,13 @@ int TableView::PreviousSelectedIndex(int item) {
   // For some reason
   // ListView_GetNextItem(list_view_,item, LVNI_SELECTED | LVNI_ABOVE)
   // fails on Vista (always returns -1), so we iterate through the indices.
-  item = std::min(item, row_count);
-  while (--item >= 0 && !IsItemSelected(item));
-  return item;
+  view_index = std::min(view_index, row_count);
+  while (--view_index >= 0 && !IsItemSelected(view_to_model(view_index)));
+  return view_index;
 }
 
-int TableView::LastSelectedIndex() {
-  return PreviousSelectedIndex(RowCount());
+int TableView::LastSelectedViewIndex() {
+  return PreviousSelectedViewIndex(RowCount());
 }
 
 void TableView::UpdateContentOffset() {
@@ -991,31 +1260,42 @@ void TableView::UpdateContentOffset() {
 // TableSelectionIterator
 //
 TableSelectionIterator::TableSelectionIterator(TableView* view,
-                                               int index)
-    : table_view_(view), index_(index) {
+                                               int view_index)
+    : table_view_(view),
+      view_index_(view_index) {
+  UpdateModelIndexFromViewIndex();
 }
 
 TableSelectionIterator& TableSelectionIterator::operator=(
     const TableSelectionIterator& other) {
-  index_ = other.index_;
+  view_index_ = other.view_index_;
+  model_index_ = other.model_index_;
   return *this;
 }
 
 bool TableSelectionIterator::operator==(const TableSelectionIterator& other) {
-  return (other.index_ == index_);
+  return (other.view_index_ == view_index_);
 }
 
 bool TableSelectionIterator::operator!=(const TableSelectionIterator& other) {
-  return (other.index_ != index_);
+  return (other.view_index_ != view_index_);
 }
 
 TableSelectionIterator& TableSelectionIterator::operator++() {
-  index_ = table_view_->PreviousSelectedIndex(index_);
+  view_index_ = table_view_->PreviousSelectedViewIndex(view_index_);
+  UpdateModelIndexFromViewIndex();
   return *this;
 }
 
 int TableSelectionIterator::operator*() {
-  return index_;
+  return model_index_;
+}
+
+void TableSelectionIterator::UpdateModelIndexFromViewIndex() {
+  if (view_index_ == -1)
+    model_index_ = -1;
+  else
+    model_index_ = table_view_->view_to_model(view_index_);
 }
 
 }  // namespace
diff --git a/chrome/views/table_view.h b/chrome/views/table_view.h
index 30f28f8..a2cfc40 100644
--- a/chrome/views/table_view.h
+++ b/chrome/views/table_view.h
@@ -22,8 +22,22 @@ class SkBitmap;
 // to display. TableModel also has an Observer which is used to notify
 // TableView of changes to the model so that the display may be updated
 // appropriately.
+//
 // TableView itself has an observer that is notified when the selection
 // changes.
+//
+// Tables may be sorted either by directly invoking SetSortDescriptors or by
+// marking the column as sortable and the user doing a gesture to sort the
+// contents. TableView itself maintains the sort so that the underlying model
+// isn't effected.
+//
+// When a table is sorted the model coordinates do not necessarily match the
+// view coordinates. All table methods are in terms of the model. If you need to
+// convert to view coordinates use model_to_view.
+//
+// Sorting is done by a locale sensitive string sort. You can customize the
+// sort by way of overriding CompareValues.
+//
 // TableView is a wrapper around the window type ListView in report mode.
 namespace ChromeViews {
 
@@ -125,6 +139,14 @@ class TableModel {
   // Sets the observer for the model. The TableView should NOT take ownership
   // of the observer.
   virtual void SetObserver(TableModelObserver* observer) = 0;
+
+  // Compares the values in the column with id |column_id| for the two rows.
+  // Returns a value < 0, == 0 or > 0 as to whether the first value is
+  // <, == or > the second value.
+  //
+  // This implementation does a case insensitive locale specific string
+  // comparison.
+  virtual int CompareValues(int row1, int row2, int column_id);
 };
 
 // TableColumn specifies the title, alignment and size of a particular column.
@@ -139,7 +161,8 @@ struct TableColumn {
         alignment(LEFT),
         width(-1),
         percent(),
-        min_visible_width(0) {
+        min_visible_width(0),
+        sortable(false) {
   }
   TableColumn(int id, const std::wstring title, Alignment alignment, int width)
       : id(id),
@@ -147,7 +170,8 @@ struct TableColumn {
         alignment(alignment),
         width(width),
         percent(0),
-        min_visible_width(0) {
+        min_visible_width(0),
+        sortable(false) {
   }
   TableColumn(int id, const std::wstring title, Alignment alignment, int width,
               float percent)
@@ -156,7 +180,8 @@ struct TableColumn {
         alignment(alignment),
         width(width),
         percent(percent),
-        min_visible_width(0) {
+        min_visible_width(0),
+        sortable(false) {
   }
   // It's common (but not required) to use the title's IDS_* tag as the column
   // id. In this case, the provided conveniences look up the title string on
@@ -166,7 +191,8 @@ struct TableColumn {
         alignment(alignment),
         width(width),
         percent(0),
-        min_visible_width(0) {
+        min_visible_width(0),
+        sortable(false) {
     title = l10n_util::GetString(id);
   }
   TableColumn(int id, Alignment alignment, int width, float percent)
@@ -174,7 +200,8 @@ struct TableColumn {
         alignment(alignment),
         width(width),
         percent(percent),
-        min_visible_width(0) {
+        min_visible_width(0),
+        sortable(false) {
     title = l10n_util::GetString(id);
   }
 
@@ -207,12 +234,15 @@ struct TableColumn {
   // (including the header)
   // to be visible.
   int min_visible_width;
+
+  // Is this column sortable? Default is false
+  bool sortable;
 };
 
 // Returned from SelectionBegin/SelectionEnd
 class TableSelectionIterator {
  public:
-  TableSelectionIterator(TableView* view, int index);
+  TableSelectionIterator(TableView* view, int view_index);
   TableSelectionIterator& operator=(const TableSelectionIterator& other);
   bool operator==(const TableSelectionIterator& other);
   bool operator!=(const TableSelectionIterator& other);
@@ -220,8 +250,14 @@ class TableSelectionIterator {
   int operator*();
 
  private:
+  void UpdateModelIndexFromViewIndex();
+
   TableView* table_view_;
-  int index_;
+  int view_index_;
+
+  // The index in terms of the model. This is returned from the * operator. This
+  // is cached to avoid dependencies on the view_to_model mapping.
+  int model_index_;
 };
 
 // TableViewObserver is notified about the TableView selection.
@@ -251,6 +287,22 @@ class TableView : public NativeControl,
     SkColor color;
   };
 
+  // Describes a sorted column.
+  struct SortDescriptor {
+    SortDescriptor() : column_id(-1), ascending(true) {}
+    SortDescriptor(int column_id, bool ascending)
+        : column_id(column_id),
+          ascending(ascending) { }
+
+    // ID of the sorted column.
+    int column_id;
+
+    // Is the sort ascending?
+    bool ascending;
+  };
+
+  typedef std::vector<SortDescriptor> SortDescriptors;
+
   // Creates a new table using the model and columns specified.
   // The table type applies to the content of the first column (text, icon and
   // text, checkbox and text).
@@ -276,6 +328,12 @@ class TableView : public NativeControl,
   // issues when the model needs to be deleted before the table.
   void SetModel(TableModel* model);
 
+  // Resorts the contents.
+  void SetSortDescriptors(const SortDescriptors& sort_descriptors);
+
+  // Current sort.
+  const SortDescriptors& sort_descriptors() const { return sort_descriptors_; }
+
   void DidChangeBounds(const CRect& previous, const CRect& current);
 
   // Returns the number of rows in the TableView.
@@ -285,28 +343,30 @@ class TableView : public NativeControl,
   int SelectedRowCount();
 
   // Selects the specified item, making sure it's visible.
-  void Select(int item);
+  void Select(int model_row);
 
   // Sets the selected state of an item (without sending any selection
   // notifications). Note that this routine does NOT set the focus to the
   // item at the given index.
-  void SetSelectedState(int item, bool state);
+  void SetSelectedState(int model_row, bool state);
 
   // Sets the focus to the item at the given index.
-  void SetFocusOnItem(int item);
+  void SetFocusOnItem(int model_row);
 
-  // Returns the first selected row.
+  // Returns the first selected row in terms of the model.
   int FirstSelectedRow();
 
   // Returns true if the item at the specified index is selected.
-  bool IsItemSelected(int item);
+  bool IsItemSelected(int model_row);
 
   // Returns true if the item at the specified index has the focus.
-  bool ItemHasTheFocus(int item);
+  bool ItemHasTheFocus(int model_row);
 
   // Returns an iterator over the selection. The iterator proceeds from the
-  // last index to the first. Do NOT use the iterator after you've mutated
-  // the model.
+  // last index to the first.
+  //
+  // NOTE: the iterator iterates over the visual order (but returns coordinates
+  // in terms of the model).
   iterator SelectionBegin();
   iterator SelectionEnd();
 
@@ -342,6 +402,19 @@ class TableView : public NativeControl,
   void SetPreferredSize(const CSize& preferred_size);
   virtual void GetPreferredSize(CSize* out);
 
+  // Is the table sorted?
+  bool is_sorted() const { return !sort_descriptors_.empty(); }
+
+  // Maps from the index in terms of the model to that of the view.
+  int model_to_view(int model_index) const {
+    return model_to_view_.get() ? model_to_view_[model_index] : model_index;
+  }
+
+  // Maps from the index in terms of the view to that of the model.
+  int view_to_model(int view_index) const {
+    return view_to_model_.get() ? view_to_model_[view_index] : view_index;
+  }
+
  protected:
   // Subclasses that want to customize the colors of a particular row/column,
   // must invoke this passing in true. The default value is false, such that
@@ -362,7 +435,7 @@ class TableView : public NativeControl,
   // Invoked to customize the colors or font at a particular cell. If you
   // change the colors or font, return true. This is only invoked if
   // SetCustomColorsEnabled(true) has been invoked.
-  virtual bool GetCellColors(int row,
+  virtual bool GetCellColors(int model_row,
                              int column,
                              ItemColor* foreground,
                              ItemColor* background,
@@ -373,7 +446,7 @@ class TableView : public NativeControl,
   // method.
   virtual bool ImplementPostPaint() { return false; }
   // Subclasses can implement in this method extra-painting for cells.
-  virtual void PostPaint(int row, int column, bool selected,
+  virtual void PostPaint(int model_row, int column, bool selected,
                          const CRect& bounds, HDC device_context) { }
 
   virtual HWND CreateNativeControl(HWND parent_container);
@@ -383,7 +456,23 @@ class TableView : public NativeControl,
   // Overriden to destroy the image list.
   virtual void OnDestroy();
 
+  // Used to sort the two rows. Returns a value < 0, == 0 or > 0 indicating
+  // whether the row2 comes before row1, row2 is the same as row1 or row1 comes
+  // after row2. This invokes CompareValues on the model with the sorted column.
+  virtual int CompareRows(int model_row1, int model_row2);
+
+  // Called before sorting. This does nothing and is intended for subclasses
+  // that need to cache state used during sorting.
+  virtual void PrepareForSort() {}
+
  private:
+  // Direction of a sort.
+  enum SortDirection {
+    ASCENDING_SORT,
+    DESCENDING_SORT,
+    NO_SORT
+  };
+
   // We need this wrapper to pass the table view to the windows proc handler
   // when subclassing the list view and list view header, as the reinterpret
   // cast from GetWindowLongPtr would break the pointer if it is pointing to a
@@ -398,6 +487,34 @@ class TableView : public NativeControl,
 
   LRESULT OnCustomDraw(NMLVCUSTOMDRAW* draw_info);
 
+  // Invoked when the user clicks on a column to toggle the sort order. If
+  // column_id is the primary sorted column the direction of the sort is
+  // toggled, otherwise column_id is made the primary sorted column.
+  void ToggleSortOrder(int column_id);
+
+  // Updates the lparam of each of the list view items to be the model index.
+  // If length is > 0, all items with an index >= start get offset by length.
+  // This is used during sorting to determine how the items were sorted.
+  void UpdateItemsLParams(int start, int length);
+
+  // Does the actual sort and updates the mappings (view_to_model and
+  // model_to_view) appropriately.
+  void SortItemsAndUpdateMapping();
+
+  // Method invoked by ListView to compare the two values. Invokes CompareRows.
+  static int CALLBACK SortFunc(LPARAM model_index_1_p,
+                               LPARAM model_index_2_p,
+                               LPARAM table_view_param);
+
+  // Method invoked by ListView when sorting back to natural state. Returns
+  // model_index_1_p - model_index_2_p.
+  static int CALLBACK NaturalSortFunc(LPARAM model_index_1_p,
+                                      LPARAM model_index_2_p,
+                                      LPARAM table_view_param);
+
+  // Resets the sort image displayed for the specified column.
+  void ResetColumnSortImage(int column_id, SortDirection direction);
+
   // Adds a new column.
   void InsertColumn(const TableColumn& tc, int index);
 
@@ -418,15 +535,19 @@ class TableView : public NativeControl,
 
   // Notification from the ListView that the checked state of the item has
   // changed.
-  void OnCheckedStateChanged(int item, bool is_checked);
+  void OnCheckedStateChanged(int model_row, bool is_checked);
 
-  // Returns the index of the selected item before |item|, or -1 if |item| is
-  // the first selected item.
-  int PreviousSelectedIndex(int item);
+  // Returns the index of the selected item before |view_index|, or -1 if
+  // |view_index| is the first selected item.
+  //
+  // WARNING: this returns coordinates in terms of the view, NOT the model.
+  int PreviousSelectedViewIndex(int view_index);
 
-  // Returns the last selected index in the table view, or -1 if the table
+  // Returns the last selected view index in the table view, or -1 if the table
   // is empty, or nothing is selected.
-  int LastSelectedIndex();
+  //
+  // WARNING: this returns coordinates in terms of the view, NOT the model.
+  int LastSelectedViewIndex();
 
   // The TableColumn visible at position pos.
   const TableColumn& GetColumnAtPosition(int pos);
@@ -460,10 +581,6 @@ class TableView : public NativeControl,
   // Cached value of columns_.size()
   int column_count_;
 
-  // Whether or not the data should be cached in the TableView.
-  // This is currently always true.
-  bool cache_data_;
-
   // Selection mode.
   bool single_selection_;
 
@@ -509,8 +626,16 @@ class TableView : public NativeControl,
   // The offset from the top of the client area to the start of the content.
   int content_offset_;
 
+  // Current sort.
+  SortDescriptors sort_descriptors_;
+
+  // Mappings used when sorted.
+  scoped_array<int> view_to_model_;
+  scoped_array<int> model_to_view_;
+
   DISALLOW_COPY_AND_ASSIGN(TableView);
 };
-}
+
+}  // namespace
 
 #endif  // CHROME_VIEWS_TABLE_VIEW_H_
diff --git a/chrome/views/table_view_unittest.cc b/chrome/views/table_view_unittest.cc
new file mode 100644
index 0000000..2467f04
--- /dev/null
+++ b/chrome/views/table_view_unittest.cc
@@ -0,0 +1,363 @@
+// Copyright (c) 2006-2008 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.
+
+#include <vector>
+
+#include "base/string_util.h"
+#include "chrome/views/table_view.h"
+#include "chrome/views/window.h"
+#include "chrome/views/window_delegate.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+using ChromeViews::TableView;
+
+// TestTableModel --------------------------------------------------------------
+
+// Trivial TableModel implementation that is backed by a vector of vectors.
+// Provides methods for adding/removing/changing the contents that notify the
+// observer appropriately.
+//
+// Initial contents are:
+// 0, 1
+// 1, 1
+// 2, 2
+class TestTableModel : public ChromeViews::TableModel {
+ public:
+  TestTableModel();
+
+  // Adds a new row at index |row| with values |c1_value| and |c2_value|.
+  void AddRow(int row, int c1_value, int c2_value);
+
+  // Removes the row at index |row|.
+  void RemoveRow(int row);
+
+  // Changes the values of the row at |row|.
+  void ChangeRow(int row, int c1_value, int c2_value);
+
+  // TableModel
+  virtual int RowCount();
+  virtual std::wstring GetText(int row, int column_id);
+  virtual void SetObserver(ChromeViews::TableModelObserver* observer);
+  virtual int CompareValues(int row1, int row2, int column_id);
+
+ private:
+  ChromeViews::TableModelObserver* observer_;
+
+  // The data.
+  std::vector<std::vector<int>> rows_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestTableModel);
+};
+
+TestTableModel::TestTableModel() : observer_(NULL) {
+  AddRow(0, 0, 1);
+  AddRow(1, 1, 1);
+  AddRow(2, 2, 2);
+}
+
+void TestTableModel::AddRow(int row, int c1_value, int c2_value) {
+  DCHECK(row >= 0 && row <= static_cast<int>(rows_.size()));
+  std::vector<int> new_row;
+  new_row.push_back(c1_value);
+  new_row.push_back(c2_value);
+  rows_.insert(rows_.begin() + row, new_row);
+  if (observer_)
+    observer_->OnItemsAdded(row, 1);
+}
+void TestTableModel::RemoveRow(int row) {
+  DCHECK(row >= 0 && row <= static_cast<int>(rows_.size()));
+  rows_.erase(rows_.begin() + row);
+  if (observer_)
+    observer_->OnItemsRemoved(row, 1);
+}
+
+void TestTableModel::ChangeRow(int row, int c1_value, int c2_value) {
+  DCHECK(row >= 0 && row < static_cast<int>(rows_.size()));
+  rows_[row][0] = c1_value;
+  rows_[row][1] = c2_value;
+  if (observer_)
+    observer_->OnItemsChanged(row, 1);
+}
+
+int TestTableModel::RowCount() {
+  return static_cast<int>(rows_.size());
+}
+ 
+std::wstring TestTableModel::GetText(int row, int column_id) {
+  return IntToWString(rows_[row][column_id]);
+}
+
+void TestTableModel::SetObserver(ChromeViews::TableModelObserver* observer) {
+  observer_ = observer;
+}
+
+int TestTableModel::CompareValues(int row1, int row2, int column_id) {
+  return rows_[row1][column_id] - rows_[row2][column_id];
+}
+
+// TableViewTest ---------------------------------------------------------------
+
+class TableViewTest : public testing::Test, ChromeViews::WindowDelegate {
+ public:
+  virtual void SetUp();
+  virtual void TearDown();
+
+  virtual ChromeViews::View* GetContentsView() {
+    return table_;
+  }
+
+ protected:
+  // Creates the model.
+  TestTableModel* CreateModel();
+
+  // Verifies the view order matches that of the supplied arguments. The
+  // arguments are in terms of the model. For example, values of '1, 0' indicate
+  // the model index at row 0 is 1 and the model index at row 1 is 0.
+  void VeriyViewOrder(int first, ...);
+
+  // Verifies the selection matches the supplied arguments. The supplied
+  // arguments are in terms of this model. This uses the iterator returned by
+  // SelectionBegin.
+  void VerifySelectedRows(int first, ...);
+
+  // Configures the state for the various multi-selection tests.
+  // This selects model rows 0 and 1, and if |sort| is true the first column
+  // is sorted in descending order.
+  void SetUpMultiSelectTestState(bool sort);
+
+  scoped_ptr<TestTableModel> model_;
+
+  // The table. This is owned by the window.
+  TableView* table_;
+
+ private:
+  MessageLoopForUI message_loop_;
+  ChromeViews::Window* window_;
+};
+
+void TableViewTest::SetUp() {
+  model_.reset(CreateModel());
+  std::vector<ChromeViews::TableColumn> columns;
+  columns.resize(2);
+  columns[0].id = 0;
+  columns[1].id = 1;
+  table_ = new TableView(model_.get(), columns, ChromeViews::ICON_AND_TEXT,
+                         false, false, false);
+  window_ =
+      ChromeViews::Window::CreateChromeWindow(NULL,
+                                              gfx::Rect(100, 100, 512, 512),
+                                              this);
+}
+
+void TableViewTest::TearDown() {
+  window_->CloseNow();
+  // Temporary workaround to avoid leak of RootView::pending_paint_task_.
+  message_loop_.RunAllPending();
+}
+
+void TableViewTest::VeriyViewOrder(int first, ...) {
+  va_list marker;
+  va_start(marker, first);
+  int value = first;
+  int index = 0;
+  for (int value = first, index = 0; value != -1; index++) {
+    ASSERT_EQ(value, table_->view_to_model(index));
+    value = va_arg(marker, int);
+  }
+  va_end(marker);
+}
+
+void TableViewTest::VerifySelectedRows(int first, ...) {
+  va_list marker;
+  va_start(marker, first);
+  int value = first;
+  int index = 0;
+  TableView::iterator selection_iterator = table_->SelectionBegin();
+  for (int value = first, index = 0; value != -1; index++) {
+    ASSERT_TRUE(selection_iterator != table_->SelectionEnd());
+    ASSERT_EQ(value, *selection_iterator);
+    value = va_arg(marker, int);
+    ++selection_iterator;
+  }
+  ASSERT_TRUE(selection_iterator == table_->SelectionEnd());
+  va_end(marker);
+}
+
+void TableViewTest::SetUpMultiSelectTestState(bool sort) {
+  // Select two rows.
+  table_->SetSelectedState(0, true);
+  table_->SetSelectedState(1, true);
+
+  VerifySelectedRows(1, 0, -1);
+  if (!sort || HasFatalFailure())
+    return;
+
+  // Sort by first column descending.
+  TableView::SortDescriptors sd;
+  sd.push_back(TableView::SortDescriptor(0, false));
+  table_->SetSortDescriptors(sd);
+  VeriyViewOrder(2, 1, 0, -1);
+  if (HasFatalFailure())
+    return;
+
+  // Make sure the two rows are sorted.
+  // NOTE: the order changed because iteration happens over view indices.
+  VerifySelectedRows(0, 1, -1);
+}
+
+TestTableModel* TableViewTest::CreateModel() {
+  return new TestTableModel();
+}
+
+// Tests -----------------------------------------------------------------------
+
+// Tests various sorting permutations.
+TEST_F(TableViewTest, Sort) {
+  // Sort by first column descending.
+  TableView::SortDescriptors sort;
+  sort.push_back(TableView::SortDescriptor(0, false));
+  table_->SetSortDescriptors(sort);
+  VeriyViewOrder(2, 1, 0, -1);
+  if (HasFatalFailure())
+    return;
+
+  // Sort by second column ascending, first column descending.
+  sort.clear();
+  sort.push_back(TableView::SortDescriptor(1, true));
+  sort.push_back(TableView::SortDescriptor(0, false));
+  sort[1].ascending = false;
+  table_->SetSortDescriptors(sort);
+  VeriyViewOrder(1, 0, 2, -1);
+  if (HasFatalFailure())
+    return;
+
+  // Clear the sort.
+  table_->SetSortDescriptors(TableView::SortDescriptors());
+  VeriyViewOrder(0, 1, 2, -1);
+  if (HasFatalFailure())
+    return;
+}
+
+// Tests changing the model while sorted.
+TEST_F(TableViewTest, SortThenChange) {
+  // Sort by first column descending.
+  TableView::SortDescriptors sort;
+  sort.push_back(TableView::SortDescriptor(0, false));
+  table_->SetSortDescriptors(sort);
+  VeriyViewOrder(2, 1, 0, -1);
+  if (HasFatalFailure())
+    return;
+
+  model_->ChangeRow(0, 3, 1);
+  VeriyViewOrder(0, 2, 1, -1);
+}
+
+// Tests adding to the model while sorted.
+TEST_F(TableViewTest, AddToSorted) {
+  // Sort by first column descending.
+  TableView::SortDescriptors sort;
+  sort.push_back(TableView::SortDescriptor(0, false));
+  table_->SetSortDescriptors(sort);
+  VeriyViewOrder(2, 1, 0, -1);
+  if (HasFatalFailure())
+    return;
+
+  // Add row so that it occurs first.
+  model_->AddRow(0, 5, -1);
+  VeriyViewOrder(0, 3, 2, 1, -1);
+  if (HasFatalFailure())
+    return;
+
+  // Add row so that it occurs last.
+  model_->AddRow(0, -1, -1);
+  VeriyViewOrder(1, 4, 3, 2, 0, -1);
+}
+
+// Tests selection on sort.
+TEST_F(TableViewTest, PersistSelectionOnSort) {
+  // Select row 0.
+  table_->Select(0);
+
+  // Sort by first column descending.
+  TableView::SortDescriptors sort;
+  sort.push_back(TableView::SortDescriptor(0, false));
+  table_->SetSortDescriptors(sort);
+  VeriyViewOrder(2, 1, 0, -1);
+  if (HasFatalFailure())
+    return;
+
+  // Make sure 0 is still selected.
+  EXPECT_EQ(0, table_->FirstSelectedRow());
+}
+
+// Tests selection iterator with sort.
+TEST_F(TableViewTest, PersistMultiSelectionOnSort) {
+  SetUpMultiSelectTestState(true);
+}
+
+// Tests selection persists after a change when sorted with iterator.
+TEST_F(TableViewTest, PersistMultiSelectionOnChangeWithSort) {
+  SetUpMultiSelectTestState(true);
+  if (HasFatalFailure())
+    return;
+
+  model_->ChangeRow(0, 3, 1);
+
+  VerifySelectedRows(1, 0, -1);
+}
+
+// Tests selection persists after a remove when sorted with iterator.
+TEST_F(TableViewTest, PersistMultiSelectionOnRemoveWithSort) {
+  SetUpMultiSelectTestState(true);
+  if (HasFatalFailure())
+    return;
+
+  model_->RemoveRow(0);
+
+  VerifySelectedRows(0, -1);
+}
+
+// Tests selection persists after a add when sorted with iterator.
+TEST_F(TableViewTest, PersistMultiSelectionOnAddWithSort) {
+  SetUpMultiSelectTestState(true);
+  if (HasFatalFailure())
+    return;
+
+  model_->AddRow(3, 4, 4);
+
+  VerifySelectedRows(0, 1, -1);
+}
+
+// Tests selection persists after a change with iterator.
+TEST_F(TableViewTest, PersistMultiSelectionOnChange) {
+  SetUpMultiSelectTestState(false);
+  if (HasFatalFailure())
+    return;
+
+  model_->ChangeRow(0, 3, 1);
+
+  VerifySelectedRows(1, 0, -1);
+}
+
+// Tests selection persists after a remove with iterator.
+TEST_F(TableViewTest, PersistMultiSelectionOnRemove) {
+  SetUpMultiSelectTestState(false);
+  if (HasFatalFailure())
+    return;
+
+  model_->RemoveRow(0);
+
+  VerifySelectedRows(0, -1);
+}
+
+// Tests selection persists after a add with iterator.
+TEST_F(TableViewTest, PersistMultiSelectionOnAdd) {
+  SetUpMultiSelectTestState(false);
+  if (HasFatalFailure())
+    return;
+
+  model_->AddRow(3, 4, 4);
+
+  VerifySelectedRows(1, 0, -1);
+}
diff --git a/chrome/views/views.vcproj b/chrome/views/views.vcproj
index ff56556..9f73219 100644
--- a/chrome/views/views.vcproj
+++ b/chrome/views/views.vcproj
@@ -18,7 +18,7 @@
 		<Configuration
 			Name="Debug|Win32"
 			ConfigurationType="4"
-			InheritedPropertySheets=".\views.vsprops;$(SolutionDir)..\build\debug.vsprops;..\tools\build\win\precompiled_wtl.vsprops"
+			InheritedPropertySheets=".\views.vsprops;$(SolutionDir)..\build\debug.vsprops;..\tools\build\win\precompiled_wtl.vsprops;..\..\third_party\icu38\build\using_icu.vsprops"
 			>
 			<Tool
 				Name="VCPreBuildEventTool"
@@ -69,7 +69,7 @@
 		<Configuration
 			Name="Release|Win32"
 			ConfigurationType="4"
-			InheritedPropertySheets=".\views.vsprops;$(SolutionDir)..\build\release.vsprops"
+			InheritedPropertySheets=".\views.vsprops;$(SolutionDir)..\build\release.vsprops;..\..\third_party\icu38\build\using_icu.vsprops"
 			>
 			<Tool
 				Name="VCPreBuildEventTool"