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

Issue 154083008: Remove Tabpose feature on mac, and supporting infrastructure (PaintAtSize) (Closed)

Created:
6 years, 10 months ago by piman
Modified:
6 years, 10 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, oshima+watch_chromium.org
Visibility:
Public.

Description

Remove Tabpose feature on mac, and supporting infrastructure (PaintAtSize) Tabpose has been behind a flag for 3 years, and UMA shows usage is very low. It is the unique user of the RenderWidgetSnapshotTaker, and the only user of RWH::PaintAtSize, which we want to remove. XIB changes: * Remove Tabpose menu entry "Show Tab Overview..." BUG=251960, 223336 R=cevans@chromium.org, jamesr@chromium.org, sky@chromium.org, viettrungluu@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251439

Patch Set 1 #

Patch Set 2 : remove tests #

Patch Set 3 : compile failure #

Patch Set 4 : removed menu entry #

Patch Set 5 : rebase #

Patch Set 6 : rebase #

Patch Set 7 : rebase #

Patch Set 8 : try again #

Patch Set 9 : try again #

Patch Set 10 : try again #

Total comments: 3

Patch Set 11 : rebase, merge #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -2842 lines) Patch
build/ios/grit_whitelist.txt View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/app/chrome_command_ids.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -1 line 0 comments Download
M chrome/app/generated_resources.grd View 2 chunks +0 lines, -9 lines 0 comments Download
M chrome/app/nibs/MainMenu.xib View 1 2 3 10 chunks +0 lines, -41 lines 0 comments Download
D chrome/app/theme/default_100_percent/common/tabpose_close.png View Binary file 0 comments Download
D chrome/app/theme/default_100_percent/mac/tabpose_close.png View Binary file 0 comments Download
D chrome/app/theme/default_200_percent/mac/tabpose_close.png View Binary file 0 comments Download
M chrome/app/theme/theme_resources.grd View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -12 lines 0 comments Download
M chrome/browser/app_controller_mac.h View 1 chunk +0 lines, -3 lines 0 comments Download
chrome/browser/app_controller_mac.mm View 1 chunk +0 lines, -5 lines 0 comments Download
chrome/browser/browser_process.h View 2 chunks +0 lines, -3 lines 0 comments Download
chrome/browser/browser_process_impl.h View 2 chunks +0 lines, -5 lines 0 comments Download
chrome/browser/browser_process_impl.cc View 3 chunks +0 lines, -6 lines 0 comments Download
D chrome/browser/thumbnails/render_widget_snapshot_taker.h View 1 chunk +0 lines, -89 lines 0 comments Download
D chrome/browser/thumbnails/render_widget_snapshot_taker.cc View 1 chunk +0 lines, -195 lines 0 comments Download
D chrome/browser/thumbnails/render_widget_snapshot_taker_unittest.cc View 1 chunk +0 lines, -86 lines 0 comments Download
chrome/browser/thumbnails/thumbnail_tab_helper.cc View 2 chunks +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/browser_command_controller.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +0 lines, -5 lines 0 comments Download
chrome/browser/ui/browser_commands.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/browser_commands.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -14 lines 0 comments Download
M chrome/browser/ui/browser_window.h View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/cocoa/accelerators_cocoa.mm View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.mm View 1 2 3 4 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_controller.h View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_controller.mm View 1 2 3 4 3 chunks +0 lines, -43 lines 0 comments Download
chrome/browser/ui/cocoa/tabpose_window.h View 1 chunk +0 lines, -103 lines 0 comments Download
chrome/browser/ui/cocoa/tabpose_window.mm View 1 chunk +0 lines, -1664 lines 0 comments Download
D chrome/browser/ui/cocoa/tabpose_window_unittest.mm View 1 chunk +0 lines, -126 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/chrome_browser_ui.gypi View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 5 6 7 8 9 10 2 chunks +0 lines, -2 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/test/base/test_browser_window.h View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/test/base/testing_browser_process.h View 2 chunks +0 lines, -2 lines 0 comments Download
M chrome/test/base/testing_browser_process.cc View 3 chunks +0 lines, -14 lines 0 comments Download
M chrome/test/base/ui_test_utils.h View 1 2 3 4 1 chunk +0 lines, -6 lines 0 comments Download
M chrome/test/base/ui_test_utils.cc View 1 2 3 4 5 6 7 8 9 10 1 chunk +0 lines, -1 line 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.h View 1 2 3 4 5 6 7 8 9 10 2 chunks +0 lines, -5 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_impl.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +0 lines, -20 lines 0 comments Download
M content/browser/renderer_host/render_widget_host_unittest.cc View 1 4 chunks +0 lines, -62 lines 0 comments Download
M content/common/swapped_out_messages.cc View 1 chunk +0 lines, -1 line 0 comments Download
M content/common/view_messages.h View 1 2 3 4 2 chunks +0 lines, -21 lines 0 comments Download
M content/public/browser/notification_types.h View 1 chunk +0 lines, -5 lines 0 comments Download
M content/public/browser/render_widget_host.h View 1 chunk +0 lines, -14 lines 0 comments Download
M content/public/test/render_widget_test.h View 1 2 1 chunk +0 lines, -24 lines 0 comments Download
M content/public/test/render_widget_test.cc View 1 1 chunk +0 lines, -116 lines 0 comments Download
M content/renderer/render_widget.h View 1 chunk +0 lines, -4 lines 0 comments Download
M content/renderer/render_widget.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +0 lines, -81 lines 0 comments Download
M content/renderer/render_widget_browsertest.cc View 1 1 chunk +0 lines, -4 lines 0 comments Download
M ui/surface/transport_dib.h View 1 3 chunks +0 lines, -21 lines 0 comments Download

Messages

Total messages: 24 (0 generated)
piman
I want to throw it out there. Updating the feature to use some common snapshotting ...
6 years, 10 months ago (2014-02-12 01:33:34 UTC) #1
Nico
I think this is the right thing to do. I considered it a few times ...
6 years, 10 months ago (2014-02-12 15:56:31 UTC) #2
Nico
On 2014/02/12 01:33:34, piman wrote: > I want to throw it out there. > Updating ...
6 years, 10 months ago (2014-02-12 16:34:58 UTC) #3
piman
Ok, I added the XIB changes to remove the menu entry. I /think/ I didn't ...
6 years, 10 months ago (2014-02-12 23:22:39 UTC) #4
Chris Evans
On 2014/02/12 23:22:39, piman wrote: > Ok, I added the XIB changes to remove the ...
6 years, 10 months ago (2014-02-12 23:25:16 UTC) #5
piman
On Wed, Feb 12, 2014 at 3:25 PM, <cevans@chromium.org> wrote: > On 2014/02/12 23:22:39, piman ...
6 years, 10 months ago (2014-02-12 23:31:07 UTC) #6
jamesr
content/ lgtm
6 years, 10 months ago (2014-02-13 00:00:47 UTC) #7
Chris Evans
On 2014/02/13 00:00:47, jamesr wrote: > content/ lgtm view_messages.h lgtm
6 years, 10 months ago (2014-02-13 00:53:07 UTC) #8
sky
Can you try uploading again, not all side-by-side diffs made it. On Wed, Feb 12, ...
6 years, 10 months ago (2014-02-13 01:35:36 UTC) #9
piman
On 2014/02/13 01:35:36, sky wrote: > Can you try uploading again, not all side-by-side diffs ...
6 years, 10 months ago (2014-02-13 01:49:34 UTC) #10
sky
LGTM https://codereview.chromium.org/154083008/diff/600001/chrome/app/chrome_command_ids.h File chrome/app/chrome_command_ids.h (left): https://codereview.chromium.org/154083008/diff/600001/chrome/app/chrome_command_ids.h#oldcode64 chrome/app/chrome_command_ids.h:64: #define IDC_TABPOSE 34036 Is there a reason why ...
6 years, 10 months ago (2014-02-13 16:59:51 UTC) #11
piman
https://codereview.chromium.org/154083008/diff/600001/chrome/app/chrome_command_ids.h File chrome/app/chrome_command_ids.h (left): https://codereview.chromium.org/154083008/diff/600001/chrome/app/chrome_command_ids.h#oldcode64 chrome/app/chrome_command_ids.h:64: #define IDC_TABPOSE 34036 On 2014/02/13 16:59:52, sky wrote: > ...
6 years, 10 months ago (2014-02-13 19:11:49 UTC) #12
piman
-rsesek (who's away too) +viettrungluu who reviewed some patches that introduced the feature
6 years, 10 months ago (2014-02-13 20:30:27 UTC) #13
viettrungluu
On 2014/02/13 20:30:27, piman wrote: > -rsesek (who's away too) +viettrungluu who reviewed some patches ...
6 years, 10 months ago (2014-02-14 16:50:15 UTC) #14
Avi (use Gerrit)
https://codereview.chromium.org/154083008/diff/600001/content/public/browser/notification_types.h File content/public/browser/notification_types.h (left): https://codereview.chromium.org/154083008/diff/600001/content/public/browser/notification_types.h#oldcode185 content/public/browser/notification_types.h:185: NOTIFICATION_RENDER_WIDGET_HOST_DID_RECEIVE_PAINT_AT_SIZE_ACK, Yes! I'm adding another bug to your bug ...
6 years, 10 months ago (2014-02-14 17:01:44 UTC) #15
piman
The CQ bit was checked by piman@chromium.org
6 years, 10 months ago (2014-02-14 19:13:34 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/piman@chromium.org/154083008/600001
6 years, 10 months ago (2014-02-14 19:14:19 UTC) #17
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-14 19:47:06 UTC) #18
commit-bot: I haz the power
Retried try job too often on mac_rel for step(s) base_unittests, browser_tests, interactive_ui_tests, net_unittests, unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&number=227266
6 years, 10 months ago (2014-02-14 19:47:07 UTC) #19
piman
The CQ bit was checked by piman@chromium.org
6 years, 10 months ago (2014-02-14 20:33:47 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/piman@chromium.org/154083008/930001
6 years, 10 months ago (2014-02-14 20:35:01 UTC) #21
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 10 months ago (2014-02-14 23:39:11 UTC) #22
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=264636
6 years, 10 months ago (2014-02-14 23:39:13 UTC) #23
piman
6 years, 10 months ago (2014-02-14 23:45:30 UTC) #24
Message was sent while issue was closed.
Committed patchset #11 manually as r251439 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698