Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(372)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 126184: Move TableModel out of views/ and into app/.... (Closed)

Created:
11 years, 6 months ago by Evan Stade
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Move TableModel out of views/ and into app/. Remove stub implementation in temp_scaffolding_stubs.h Use l10n_util collator helper function in TableModel::Compare Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=18518

Patch Set 1 #

Patch Set 2 : fix whitespace #

Patch Set 3 : changes I missed #

Patch Set 4 : '' #

Total comments: 1

Patch Set 5 : addressed comments, build fixes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+178 lines, -473 lines) Patch
M app/app.gyp View 1 chunk +3 lines, -0 lines 0 comments Download
A + app/table_model.h View 1 2 3 4 chunks +6 lines, -7 lines 0 comments Download
A + app/table_model.cc View 1 2 3 4 2 chunks +41 lines, -3 lines 0 comments Download
A + app/table_model_observer.h View 1 2 3 2 chunks +3 lines, -7 lines 0 comments Download
M chrome/browser/bookmarks/bookmark_table_model.h View 3 chunks +5 lines, -10 lines 0 comments Download
M chrome/browser/bookmarks/bookmark_table_model.cc View 2 chunks +1 line, -4 lines 0 comments Download
M chrome/browser/bookmarks/bookmark_table_model_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
MM chrome/browser/task_manager_win.cc View 1 10 chunks +23 lines, -24 lines 0 comments Download
M chrome/browser/views/bookmark_table_view.cc View 1 3 chunks +9 lines, -10 lines 0 comments Download
M chrome/browser/views/hung_renderer_view.cc View 3 chunks +5 lines, -5 lines 0 comments Download
M chrome/browser/views/keyword_editor_view.h View 4 chunks +4 lines, -4 lines 0 comments Download
M chrome/browser/views/keyword_editor_view.cc View 2 chunks +1 line, -2 lines 0 comments Download
chrome/browser/views/keyword_editor_view_unittest.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/views/options/cookies_view.cc View 1 11 chunks +15 lines, -15 lines 0 comments Download
MM chrome/browser/views/options/exceptions_page_view.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/views/options/fonts_page_view.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/views/options/general_page_view.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/views/options/general_page_view.cc View 5 chunks +7 lines, -8 lines 0 comments Download
M chrome/browser/views/options/languages_page_view.h View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/views/options/languages_page_view.cc View 7 chunks +9 lines, -10 lines 0 comments Download
MM chrome/browser/views/options/passwords_page_view.h View 6 chunks +6 lines, -6 lines 0 comments Download
chrome/browser/views/options/passwords_page_view.cc View 1 2 chunks +6 lines, -7 lines 0 comments Download
M chrome/browser/views/shelf_item_dialog.cc View 6 chunks +8 lines, -8 lines 0 comments Download
M chrome/common/temp_scaffolding_stubs.h View 1 chunk +0 lines, -17 lines 0 comments Download
M views/controls/table/group_table_view.h View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
D views/controls/table/table_model.h View 1 chunk +0 lines, -154 lines 0 comments Download
D views/controls/table/table_model.cc View 1 chunk +0 lines, -68 lines 0 comments Download
D views/controls/table/table_model_observer.h View 1 chunk +0 lines, -29 lines 0 comments Download
M views/controls/table/table_view.h View 1 2 3 4 2 chunks +3 lines, -3 lines 0 comments Download
M views/controls/table/table_view.cc View 1 2 3 3 chunks +3 lines, -50 lines 0 comments Download
M views/controls/table/table_view_unittest.cc View 5 chunks +7 lines, -6 lines 0 comments Download
M views/views.gyp View 1 chunk +0 lines, -3 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Evan Stade
11 years, 6 months ago (2009-06-16 02:26:11 UTC) #1
Ben Goodger (Google)
OK.
11 years, 6 months ago (2009-06-16 11:46:55 UTC) #2
sky
11 years, 6 months ago (2009-06-16 16:12:22 UTC) #3 
LGTM with the following change.

http://codereview.chromium.org/126184/diff/1099/107
File app/table_model.cc (right):

http://codereview.chromium.org/126184/diff/1099/107#newcode105
Line 105: if (collator)
Seems weird to conditionalize the setter but not the delete.
Do either:
  delete collator;
  collator = NULL;
or
  if (collator) {
    delete collator;
    collator = NULL;
  }
But not this.

Powered by Google App Engine
This is Rietveld 408576698