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

Issue 12211122: Reland Remove NativeTabbedPane[Win|Wrapper]; promote Views impl. (Closed)

Created:
7 years, 10 months ago by msw
Modified:
7 years, 10 months ago
Reviewers:
sky
CC:
chromium-reviews, tfarina, markusheintz_, Ben Goodger (Google)
Visibility:
Public.

Description

Reland Remove NativeTabbedPane[Win|Wrapper]; promote Views impl. Original CL (reverted for leaks): https://codereview.chromium.org/12225042/ http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20Heapcheck/builds/14990 http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20%28valgrind%29%283%29/builds/22339 Leaks are fixed here by having the tabbed pane own its tab content Views. (all Valgrind and Heapcheck leaks observed locally are fixed by this change) TBR=sky@chromium.org The original CL description is copied below: ======================================== Remove NativeTabbedPane[Win|Wrapper]; promote Views impl. Remove NativeTabbedPaneWin and NativeTabbedPaneWrapper. ( its last use was eliminated in http://crrev.com/180924 ) Write a TabbedPane based on NativeTabbedPaneViews. Update unit tests; cleanup; etc. BUG=138059 TEST=No observable tabbed pane appearance/behavior changes (see the WebsiteSettingsPopupView and CollectedCookiesView); unit tests. R=markusheintz@chromium.org,sky@chromium.org,ben@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181945

Patch Set 1 #

Patch Set 2 : Fix leak of tab content Views. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+358 lines, -1454 lines) Patch
M chrome/browser/ui/views/collected_cookies_views.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/website_settings/website_settings_popup_view.cc View 1 chunk +2 lines, -4 lines 0 comments Download
D ui/views/controls/tabbed_pane/native_tabbed_pane_views.h View 1 chunk +0 lines, -74 lines 0 comments Download
D ui/views/controls/tabbed_pane/native_tabbed_pane_views.cc View 1 chunk +0 lines, -450 lines 0 comments Download
D ui/views/controls/tabbed_pane/native_tabbed_pane_win.h View 1 chunk +0 lines, -92 lines 0 comments Download
D ui/views/controls/tabbed_pane/native_tabbed_pane_win.cc View 1 chunk +0 lines, -398 lines 0 comments Download
D ui/views/controls/tabbed_pane/native_tabbed_pane_wrapper.h View 1 chunk +0 lines, -74 lines 0 comments Download
M ui/views/controls/tabbed_pane/tabbed_pane.h View 3 chunks +21 lines, -45 lines 0 comments Download
M ui/views/controls/tabbed_pane/tabbed_pane.cc View 1 2 chunks +289 lines, -87 lines 0 comments Download
M ui/views/controls/tabbed_pane/tabbed_pane_unittest.cc View 4 chunks +29 lines, -119 lines 0 comments Download
M ui/views/examples/tabbed_pane_example.h View 2 chunks +2 lines, -3 lines 0 comments Download
M ui/views/examples/tabbed_pane_example.cc View 5 chunks +3 lines, -8 lines 0 comments Download
M ui/views/focus/focus_manager_unittest.cc View 3 chunks +3 lines, -59 lines 0 comments Download
M ui/views/focus/focus_traversal_unittest.cc View 12 chunks +8 lines, -35 lines 0 comments Download
M ui/views/views.gyp View 1 chunk +0 lines, -5 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
msw
I'll land this unless there is any additional feedback/comments; thanks!
7 years, 10 months ago (2013-02-12 06:56:48 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/msw@chromium.org/12211122/3002
7 years, 10 months ago (2013-02-12 16:15:29 UTC) #2
commit-bot: I haz the power
7 years, 10 months ago (2013-02-12 17:38:53 UTC) #3
Message was sent while issue was closed.
Change committed as 181945

Powered by Google App Engine
This is Rietveld 408576698