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

Issue 10911074: Change how ui::Clipboard is accessed so there's only one per thread. (Closed)

Created:
8 years, 3 months ago by Elliot Glaysher
Modified:
8 years, 3 months ago
CC:
chromium-reviews, Avi (use Gerrit), jam, creis+watch_chromium.org, hashimoto+watch_chromium.org, aboxhall+watch_chromium.org, tfarina, browser-components-watch_chromium.org, yuzo+watch_chromium.org, davidbarr+watch_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org, ajwong+watch_chromium.org, pam+watch_chromium.org, dcheng, ctguil+watch_chromium.org, yoshiki+watch_chromium.org, zork+watch_chromium.org
Visibility:
Public.

Description

Change how ui::Clipboard is accessed so there's only one per thread. Currently, there can be any number of Clipboard objects, which can be massively simplified. This removes interfaces for fetching the Clipboard and makes everyone go through a single static ui::Clipboard::GetForCurrentThread() access point. BUG=130805 TBR=tc (change in webkit/ is trivial) Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=155468

Patch Set 1 #

Patch Set 2 : Fix stupid compile everywhere failure. #

Patch Set 3 : Mass sed rename to remove the views_delegate access. #

Patch Set 4 : More dumb compile problems. #

Patch Set 5 : Fix for windows compile #

Patch Set 6 : Patch cleanup #

Total comments: 2

Patch Set 7 : Change deletion semantics #

Patch Set 8 : Fix various windows compile failures. #

Total comments: 3

Patch Set 9 : views_delegate.h deletion #

Total comments: 4

Patch Set 10 : Maybe fix windows tests #

Patch Set 11 : Fixes for kaiwang #

Unified diffs Side-by-side diffs Delta from patch set Stats (+221 lines, -297 lines) Patch
ash/drag_drop/drag_drop_controller_unittest.cc View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
chrome/browser/bookmarks/bookmark_node_data.cc View 1 2 3 4 5 6 7 8 9 10 4 chunks +3 lines, -5 lines 0 comments Download
chrome/browser/bookmarks/bookmark_utils_unittest.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +3 lines, -6 lines 0 comments Download
chrome/browser/browser_process.h View 2 chunks +0 lines, -5 lines 0 comments Download
chrome/browser/browser_process_impl.h View 2 chunks +0 lines, -3 lines 0 comments Download
chrome/browser/browser_process_impl.cc View 1 2 3 4 5 6 7 8 9 10 3 chunks +0 lines, -7 lines 0 comments Download
chrome/browser/tab_contents/render_view_context_menu.cc View 1 2 3 4 5 6 7 8 9 2 chunks +2 lines, -1 line 0 comments Download
chrome/browser/ui/omnibox/omnibox_view.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +1 line, -2 lines 0 comments Download
chrome/browser/ui/omnibox/omnibox_view_unittest.cc View 1 2 3 4 5 7 chunks +9 lines, -9 lines 0 comments Download
chrome/browser/ui/views/accessibility/accessibility_event_router_views_unittest.cc View 1 chunk +0 lines, -1 line 0 comments Download
chrome/browser/ui/views/bookmarks/bookmark_bar_view_test.cc View 1 chunk +0 lines, -1 line 0 comments Download
chrome/browser/ui/views/chrome_views_delegate.h View 1 chunk +0 lines, -1 line 0 comments Download
chrome/browser/ui/views/chrome_views_delegate.cc View 1 2 3 2 chunks +0 lines, -5 lines 0 comments Download
chrome/browser/ui/views/find_bar_host_interactive_uitest.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
chrome/browser/ui/views/menu_model_adapter_test.cc View 1 chunk +0 lines, -4 lines 0 comments Download
chrome/browser/ui/views/omnibox/omnibox_view_views.cc View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
chrome/browser/ui/views/omnibox/omnibox_view_win.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +1 line, -2 lines 0 comments Download
chrome/test/base/testing_browser_process.h View 3 chunks +0 lines, -6 lines 0 comments Download
chrome/test/base/testing_browser_process.cc View 2 chunks +0 lines, -9 lines 0 comments Download
chrome_frame/test/chrome_frame_test_utils.cc View 1 2 3 4 5 6 7 8 1 chunk +6 lines, -8 lines 0 comments Download
content/browser/browser_main_loop.cc View 1 2 3 4 5 6 2 chunks +2 lines, -0 lines 0 comments Download
content/browser/renderer_host/clipboard_message_filter.cc View 2 chunks +3 lines, -4 lines 0 comments Download
content/shell/shell_aura.cc View 1 2 3 4 5 6 7 8 9 2 chunks +0 lines, -8 lines 0 comments Download
ui/base/clipboard/clipboard.h View 1 2 3 4 5 6 3 chunks +17 lines, -2 lines 0 comments Download
ui/base/clipboard/clipboard.cc View 1 2 3 4 5 6 3 chunks +38 lines, -0 lines 0 comments Download
ui/base/clipboard/clipboard_unittest.cc View 1 2 3 4 5 6 7 8 9 24 chunks +111 lines, -152 lines 0 comments Download
ui/views/controls/message_box_view.cc View 1 2 3 4 5 6 7 8 2 chunks +1 line, -5 lines 0 comments Download
ui/views/controls/textfield/native_textfield_views.cc View 1 2 3 4 5 6 7 8 9 2 chunks +2 lines, -3 lines 0 comments Download
ui/views/controls/textfield/native_textfield_views_unittest.cc View 1 2 3 4 5 6 7 8 9 3 chunks +3 lines, -4 lines 0 comments Download
ui/views/controls/textfield/native_textfield_win.cc View 1 2 3 4 5 6 7 8 9 3 chunks +4 lines, -5 lines 0 comments Download
ui/views/controls/textfield/textfield_views_model.cc View 1 2 3 4 5 6 7 8 4 chunks +3 lines, -4 lines 0 comments Download
ui/views/controls/textfield/textfield_views_model_unittest.cc View 1 2 3 4 5 6 7 8 2 chunks +1 line, -2 lines 0 comments Download
ui/views/test/test_views_delegate.h View 2 chunks +0 lines, -2 lines 0 comments Download
ui/views/test/test_views_delegate.cc View 2 chunks +0 lines, -9 lines 0 comments Download
ui/views/view_unittest.cc View 1 2 3 4 5 6 7 8 9 6 chunks +7 lines, -9 lines 0 comments Download
ui/views/views_delegate.h View 2 chunks +0 lines, -7 lines 0 comments Download
webkit/tools/test_shell/simple_clipboard_impl.cc View 2 chunks +1 line, -3 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
Elliot Glaysher
dcheng: clipboard master ben: views stuff jam: content stuff
8 years, 3 months ago (2012-09-05 17:32:00 UTC) #1
jam
content lgtm https://codereview.chromium.org/10911074/diff/4036/ui/base/clipboard/clipboard.cc File ui/base/clipboard/clipboard.cc (right): https://codereview.chromium.org/10911074/diff/4036/ui/base/clipboard/clipboard.cc#newcode116 ui/base/clipboard/clipboard.cc:116: delete it->second; the comment in the header ...
8 years, 3 months ago (2012-09-05 17:42:28 UTC) #2
Elliot Glaysher
https://codereview.chromium.org/10911074/diff/4036/ui/base/clipboard/clipboard.cc File ui/base/clipboard/clipboard.cc (right): https://codereview.chromium.org/10911074/diff/4036/ui/base/clipboard/clipboard.cc#newcode116 ui/base/clipboard/clipboard.cc:116: delete it->second; On 2012/09/05 17:42:28, John Abd-El-Malek wrote: > ...
8 years, 3 months ago (2012-09-05 18:07:22 UTC) #3
Ben Goodger (Google)
can you try removing the views-delegate.h headers in ui/views too? lgtm with that. https://chromiumcodereview.appspot.com/10911074/diff/12002/ui/views/controls/textfield/native_textfield_views.cc File ...
8 years, 3 months ago (2012-09-05 18:25:50 UTC) #4
dcheng
https://chromiumcodereview.appspot.com/10911074/diff/12002/ui/base/clipboard/clipboard.h File ui/base/clipboard/clipboard.h (right): https://chromiumcodereview.appspot.com/10911074/diff/12002/ui/base/clipboard/clipboard.h#newcode298 ui/base/clipboard/clipboard.h:298: friend class ClipboardTest; Out of curiosity, is this because ...
8 years, 3 months ago (2012-09-05 18:28:06 UTC) #5
Elliot Glaysher
https://chromiumcodereview.appspot.com/10911074/diff/12002/ui/base/clipboard/clipboard.h File ui/base/clipboard/clipboard.h (right): https://chromiumcodereview.appspot.com/10911074/diff/12002/ui/base/clipboard/clipboard.h#newcode298 ui/base/clipboard/clipboard.h:298: friend class ClipboardTest; On 2012/09/05 18:28:06, dcheng wrote: > ...
8 years, 3 months ago (2012-09-05 19:26:44 UTC) #6
dcheng
lgtm
8 years, 3 months ago (2012-09-05 20:28:21 UTC) #7
Elliot Glaysher
the win_rel unit test failures are probably real. pxeing a windows box to investigate.
8 years, 3 months ago (2012-09-05 22:31:15 UTC) #8
kaiwang
8 years, 3 months ago (2012-09-06 22:11:47 UTC) #9
Besides below 4, I think you may remove browser_process.h including from other
files

https://chromiumcodereview.appspot.com/10911074/diff/7041/chrome/browser/book...
File chrome/browser/bookmarks/bookmark_node_data.cc (right):

https://chromiumcodereview.appspot.com/10911074/diff/7041/chrome/browser/book...
chrome/browser/bookmarks/bookmark_node_data.cc:23: #include
"chrome/browser/browser_process.h"
Remove this include?

https://chromiumcodereview.appspot.com/10911074/diff/7041/chrome/browser/book...
File chrome/browser/bookmarks/bookmark_utils_unittest.cc (right):

https://chromiumcodereview.appspot.com/10911074/diff/7041/chrome/browser/book...
chrome/browser/bookmarks/bookmark_utils_unittest.cc:15: #endif
remove include?

https://chromiumcodereview.appspot.com/10911074/diff/7041/chrome/browser/ui/o...
File chrome/browser/ui/omnibox/omnibox_view.cc (right):

https://chromiumcodereview.appspot.com/10911074/diff/7041/chrome/browser/ui/o...
chrome/browser/ui/omnibox/omnibox_view.cc:14: #include
"chrome/browser/browser_process.h"
remove?

https://chromiumcodereview.appspot.com/10911074/diff/7041/chrome/browser/ui/v...
File chrome/browser/ui/views/omnibox/omnibox_view_win.cc (right):

https://chromiumcodereview.appspot.com/10911074/diff/7041/chrome/browser/ui/v...
chrome/browser/ui/views/omnibox/omnibox_view_win.cc:30: #include
"chrome/browser/browser_process.h"
remove?

Powered by Google App Engine
This is Rietveld 408576698