Chromium Code Reviews
Help | Chromium Project | Sign in
(1033)

Issue 342068: Third patch in getting rid of caching MessageLoop pointers and always using C... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 5 months ago by jam
Modified:
2 years, 11 months ago
Reviewers:
Evan Stade
CC:
chromium-reviews_googlegroups.com, brettw+cc_chromium.org, ncarter, idana, ben+cc_chromium.org, Erik does not do reviews, Paul Godavari, jam, Aaron Boodman, pam+watch_chromium.org, PaweĊ‚ Hajdan Jr., kuchhal, darin
Visibility:
Public.

Description

Third patch in getting rid of caching MessageLoop pointers and always using ChromeThread instead.

BUG=25354
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30687

Patch Set 1 #

Total comments: 26

Patch Set 2 : '' #

Patch Set 3 : '' #

Total comments: 2

Patch Set 4 : fix trybot build failures #

Patch Set 5 : update after review #

Patch Set 6 : fix gcc build failures #

Patch Set 7 : fix hangs on mac and linux unit_tests #

Patch Set 8 : '' #

Patch Set 9 : '' #

Patch Set 10 : '' #

Patch Set 11 : '' #

Patch Set 12 : '' #

Patch Set 13 : '' #

Patch Set 14 : '' #

Patch Set 15 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+618 lines, -716 lines) Lint Patch
M chrome/browser/autocomplete/history_contents_provider_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +6 lines, -0 lines 0 comments 0 errors Download
M chrome/browser/autocomplete/history_url_provider_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +9 lines, -0 lines 0 comments 0 errors Download
M chrome/browser/bookmarks/bookmark_drag_data_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +17 lines, -1 line 0 comments 0 errors Download
M chrome/browser/bookmarks/bookmark_folder_tree_model_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +9 lines, -1 line 0 comments ? errors Download
M chrome/browser/bookmarks/bookmark_html_writer.h View 1 2 3 1 chunk +3 lines, -6 lines 0 comments ? errors Download
M chrome/browser/bookmarks/bookmark_html_writer.cc View 1 2 3 2 chunks +4 lines, -8 lines 0 comments ? errors Download
M chrome/browser/bookmarks/bookmark_html_writer_unittest.cc View 1 2 3 2 chunks +6 lines, -1 line 0 comments ? errors Download
M chrome/browser/bookmarks/bookmark_index_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +3 lines, -0 lines 0 comments 0 errors Download
M chrome/browser/bookmarks/bookmark_model_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +6 lines, -0 lines 0 comments ? errors Download
M chrome/browser/bookmarks/bookmark_storage.h View 1 2 3 2 chunks +0 lines, -15 lines 0 comments 0 errors Download
M chrome/browser/bookmarks/bookmark_storage.cc View 1 2 3 4 5 6 7 8 8 chunks +13 lines, -32 lines 0 comments ? errors Download
M chrome/browser/bookmarks/bookmark_table_model_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +9 lines, -1 line 0 comments 0 errors Download
M chrome/browser/browser_about_handler.cc View 1 2 3 2 chunks +5 lines, -2 lines 0 comments ? errors Download
M chrome/browser/browser_commands_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +3 lines, -0 lines 0 comments 0 errors Download
M chrome/browser/browser_main.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments 0 errors Download
M chrome/browser/browser_process_impl.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments 0 errors Download
M chrome/browser/browser_shutdown.cc View 1 2 3 2 chunks +3 lines, -3 lines 0 comments 0 errors Download
M chrome/browser/browsing_data_remover.cc View 1 2 3 4 2 chunks +11 lines, -13 lines 0 comments 0 errors Download
M chrome/browser/chrome_plugin_host.cc View 1 2 3 2 chunks +3 lines, -1 line 0 comments 0 errors Download
M chrome/browser/cocoa/browser_test_helper.h View 7 8 9 10 11 12 3 chunks +6 lines, -1 line 0 comments ? errors Download
MM chrome/browser/debugger/devtools_protocol_handler.h View 1 2 3 1 chunk +0 lines, -2 lines 0 comments 0 errors Download
MM chrome/browser/debugger/devtools_protocol_handler.cc View 1 2 3 8 chunks +18 lines, -15 lines 0 comments 0 errors Download
M chrome/browser/dom_ui/new_tab_ui_uitest.cc View 1 chunk +1 line, -1 line 0 comments ? errors Download
M chrome/browser/dom_ui/shown_sections_handler_unittest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments 0 errors Download
M chrome/browser/extensions/extension_updater_unittest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments 0 errors Download
M chrome/browser/extensions/extensions_service_unittest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments 0 errors Download
M chrome/browser/gtk/bookmark_bar_gtk_unittest.cc View 8 9 10 11 12 3 chunks +8 lines, -1 line 0 comments ? errors Download
M chrome/browser/gtk/bookmark_editor_gtk_unittest.cc View 7 8 9 10 11 12 3 chunks +8 lines, -1 line 0 comments ? errors Download
M chrome/browser/gtk/bookmark_manager_gtk.cc View 1 2 3 1 chunk +1 line, -4 lines 0 comments 0 errors Download
M chrome/browser/gtk/dialogs_gtk.cc View 1 2 3 4 5 6 2 chunks +3 lines, -4 lines 0 comments 0 errors Download
M chrome/browser/jankometer.cc View 1 2 3 2 chunks +4 lines, -6 lines 0 comments 0 errors Download
M chrome/browser/memory_details.h View 1 2 3 2 chunks +0 lines, -3 lines 0 comments 0 errors Download
M chrome/browser/memory_details.cc View 1 2 3 3 chunks +5 lines, -7 lines 0 comments ? errors Download
M chrome/browser/memory_details_linux.cc View 2 chunks +3 lines, -3 lines 0 comments ? errors Download
M chrome/browser/memory_details_win.cc View 1 2 3 3 chunks +3 lines, -4 lines 0 comments ? errors Download
M chrome/browser/metrics/metrics_service_uitest.cc View 1 chunk +1 line, -2 lines 0 comments ? errors Download
M chrome/browser/net/chrome_url_request_context.cc View 1 2 3 4 5 chunks +11 lines, -5 lines 0 comments 0 errors Download
M chrome/browser/net/dns_global.cc View 1 2 3 2 chunks +0 lines, -2 lines 0 comments 0 errors Download
M chrome/browser/net/dns_master.h View 1 2 3 3 chunks +4 lines, -7 lines 0 comments 0 errors Download
M chrome/browser/net/dns_master.cc View 1 2 3 7 chunks +22 lines, -20 lines 0 comments 0 errors Download
M chrome/browser/net/dns_master_unittest.cc View 1 2 3 4 12 chunks +14 lines, -23 lines 0 comments 0 errors Download
M chrome/browser/notifications/desktop_notification_service.cc View 1 2 3 4 chunks +14 lines, -21 lines 0 comments ? errors Download
M chrome/browser/printing/print_dialog_gtk.h View 1 chunk +2 lines, -4 lines 0 comments ? errors Download
M chrome/browser/printing/print_dialog_gtk.cc View 2 chunks +4 lines, -4 lines 0 comments ? errors Download
M chrome/browser/profile.cc View 1 2 3 7 chunks +18 lines, -33 lines 0 comments 0 errors Download
M chrome/browser/profile_manager.cc View 1 2 3 1 chunk +6 lines, -4 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/browser_render_process_host.cc View 1 2 3 1 chunk +3 lines, -3 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/buffered_resource_handler.h View 1 2 3 1 chunk +2 lines, -7 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/buffered_resource_handler.cc View 1 2 3 2 chunks +14 lines, -23 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/render_widget_helper.h View 1 2 3 3 chunks +0 lines, -4 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/render_widget_helper.cc View 1 2 3 7 chunks +26 lines, -21 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/render_widget_host_view_win.cc View 1 2 3 2 chunks +3 lines, -1 line 0 comments 0 errors Download
M chrome/browser/renderer_host/resource_message_filter.h View 1 2 3 1 chunk +0 lines, -1 line 0 comments 0 errors Download
M chrome/browser/renderer_host/resource_message_filter.cc View 1 2 3 6 chunks +11 lines, -12 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/resource_message_filter_gtk.cc View 2 chunks +21 lines, -13 lines 0 comments ? errors Download
M chrome/browser/renderer_host/resource_message_filter_mac.mm View 2 chunks +3 lines, -1 line 0 comments ? errors Download
M chrome/browser/safe_browsing/safe_browsing_blocking_page.cc View 1 2 3 2 chunks +6 lines, -10 lines 0 comments 0 errors Download
M chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc View 1 2 3 4 chunks +6 lines, -1 line 0 comments 0 errors Download
M chrome/browser/safe_browsing/safe_browsing_service.cc View 1 2 3 1 chunk +4 lines, -7 lines 0 comments 0 errors Download
M chrome/browser/spellcheck_unittest.cc View 1 2 3 10 chunks +27 lines, -1 line 0 comments 0 errors Download
M chrome/browser/spellchecker.h View 1 2 3 3 chunks +8 lines, -4 lines 0 comments 0 errors Download
M chrome/browser/spellchecker.cc View 1 2 3 14 chunks +28 lines, -108 lines 0 comments 0 errors Download
M chrome/browser/strict_transport_security_persister.h View 1 2 3 4 chunks +2 lines, -10 lines 0 comments 0 errors Download
M chrome/browser/strict_transport_security_persister.cc View 1 2 3 4 chunks +6 lines, -12 lines 0 comments 0 errors Download
M chrome/browser/sync/profile_sync_service_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +7 lines, -1 line 0 comments 0 errors Download
M chrome/browser/sync/sync_setup_wizard.cc View 1 2 3 2 chunks +11 lines, -9 lines 0 comments 0 errors Download
M chrome/browser/sync/sync_setup_wizard_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +7 lines, -1 line 0 comments 0 errors Download
M chrome/browser/tab_contents/interstitial_page.cc View 1 2 3 2 chunks +9 lines, -8 lines 0 comments 0 errors Download
M chrome/browser/tab_contents/web_contents_unittest.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments 0 errors Download
M chrome/browser/task_manager.cc View 1 2 3 3 chunks +9 lines, -9 lines 0 comments 0 errors Download
M chrome/browser/task_manager_resource_providers.cc View 1 2 3 2 chunks +6 lines, -3 lines 0 comments ? errors Download
M chrome/browser/views/about_network_dialog.cc View 1 2 3 2 chunks +3 lines, -5 lines 0 comments 0 errors Download
M chrome/browser/views/bookmark_context_menu_test.cc View 1 2 3 4 5 6 7 8 9 10 11 12 3 chunks +6 lines, -1 line 0 comments 0 errors Download
M chrome/browser/views/bookmark_editor_view_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 4 chunks +9 lines, -1 line 0 comments 0 errors Download
M chrome/browser/views/bookmark_manager_view.cc View 1 2 3 1 chunk +1 line, -5 lines 0 comments 0 errors Download
M chrome/browser/visitedlink_master.h View 1 2 3 5 chunks +6 lines, -21 lines 0 comments 0 errors Download
M chrome/browser/visitedlink_master.cc View 1 2 3 9 chunks +18 lines, -45 lines 0 comments 0 errors Download
M chrome/browser/visitedlink_perftest.cc View 4 chunks +4 lines, -6 lines 0 comments ? errors Download
M chrome/browser/visitedlink_unittest.cc View 1 2 3 7 chunks +17 lines, -4 lines 0 comments 0 errors Download
M chrome/browser/web_resource/web_resource_service.h View 1 2 3 2 chunks +1 line, -5 lines 0 comments 0 errors Download
M chrome/browser/web_resource/web_resource_service.cc View 1 2 3 1 chunk +1 line, -3 lines 0 comments 0 errors Download
M chrome/browser/worker_host/worker_process_host.cc View 1 2 3 2 chunks +4 lines, -4 lines 0 comments 0 errors Download
M chrome/browser/worker_host/worker_service.h View 1 2 3 4 chunks +1 line, -6 lines 0 comments 0 errors Download
M chrome/browser/worker_host/worker_service.cc View 1 2 3 2 chunks +2 lines, -5 lines 0 comments 0 errors Download
M chrome/common/histogram_synchronizer.cc View 1 2 3 4 chunks +10 lines, -6 lines 0 comments ? errors Download
MM chrome/common/important_file_writer.h View 1 2 3 2 chunks +2 lines, -7 lines 0 comments ? errors Download
MM chrome/common/important_file_writer.cc View 1 2 3 3 chunks +4 lines, -10 lines 0 comments ? errors Download
MM chrome/common/important_file_writer_unittest.cc View 1 2 3 4 chunks +13 lines, -18 lines 0 comments ? errors Download
M chrome/common/pref_member_unittest.cc View 1 2 3 3 chunks +3 lines, -3 lines 0 comments ? errors Download
M chrome/common/pref_service.h View 1 2 3 2 chunks +2 lines, -9 lines 0 comments ? errors Download
M chrome/common/pref_service.cc View 1 2 3 2 chunks +2 lines, -4 lines 0 comments ? errors Download
M chrome/common/pref_service_unittest.cc View 1 2 3 9 chunks +13 lines, -7 lines 0 comments ? errors Download
M chrome/test/in_process_browser_test.cc View 1 2 3 2 chunks +3 lines, -1 line 0 comments 0 errors Download
M chrome/test/reliability/page_load_test.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments 0 errors Download
M chrome/test/testing_profile.h View 1 2 3 1 chunk +1 line, -1 line 0 comments 0 errors Download
Commit:

Messages

Total messages: 10
jam
Apologies this got much bigger than I thought it would.
4 years, 5 months ago #1
timsteele
http://codereview.chromium.org/342068/diff/5086/6024 File chrome/browser/sync/profile_sync_service_unittest.cc (right): http://codereview.chromium.org/342068/diff/5086/6024#newcode469 Line 469: ChromeThread file_thread_; I'm curious what this is for; ...
4 years, 5 months ago #2
jam
On Fri, Oct 30, 2009 at 3:57 PM, <tim@chromium.org> wrote: > > http://codereview.chromium.org/342068/diff/5086/6024 > File ...
4 years, 5 months ago #3
Evan Stade
mostly lg http://codereview.chromium.org/342068/diff/1/13 File chrome/browser/autocomplete/history_url_provider_unittest.cc (right): http://codereview.chromium.org/342068/diff/1/13#newcode99 Line 99: HistoryURLProviderTest() : file_thread_(ChromeThread::FILE, &message_loop_) {} nit: ...
4 years, 5 months ago #4
jam
http://codereview.chromium.org/342068/diff/1/13 File chrome/browser/autocomplete/history_url_provider_unittest.cc (right): http://codereview.chromium.org/342068/diff/1/13#newcode99 Line 99: HistoryURLProviderTest() : file_thread_(ChromeThread::FILE, &message_loop_) {} On 2009/10/30 23:21:57, ...
4 years, 5 months ago #5
Evan Stade
On Fri, Oct 30, 2009 at 4:47 PM, <jam@chromium.org> wrote: > > http://codereview.chromium.org/342068/diff/1/13 > File ...
4 years, 5 months ago #6
Evan Stade
http://codereview.chromium.org/342068/diff/1/44 File chrome/browser/gtk/dialogs_gtk.cc (right): http://codereview.chromium.org/342068/diff/1/44#newcode152 Line 152: DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::UI)); On 2009/10/30 23:47:00, John Abd-El-Malek wrote: > ...
4 years, 5 months ago #7
jam
On Fri, Oct 30, 2009 at 5:05 PM, Evan Stade <estade@chromium.org> wrote: > On Fri, ...
4 years, 5 months ago #8
jam
http://codereview.chromium.org/342068/diff/1/44 File chrome/browser/gtk/dialogs_gtk.cc (right): http://codereview.chromium.org/342068/diff/1/44#newcode152 Line 152: DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::UI)); On 2009/10/31 00:05:59, Evan Stade wrote: > ...
4 years, 5 months ago #9
Evan Stade
4 years, 5 months ago #10
On Fri, Oct 30, 2009 at 5:43 PM,  <jam@chromium.org> wrote:
>
> http://codereview.chromium.org/342068/diff/1/44
> File chrome/browser/gtk/dialogs_gtk.cc (right):
>
> http://codereview.chromium.org/342068/diff/1/44#newcode152
> Line 152: DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::UI));
> On 2009/10/31 00:05:59, Evan Stade wrote:
>>
>> On 2009/10/30 23:47:00, John Abd-El-Malek wrote:
>> > On 2009/10/30 23:21:57, Evan Stade wrote:
>> > > check should just be that we _are_ on the ui thread
>> >
>> > I just kept the old behavior. =A0I believe it's doing this to succeed
>
> in
>>
>> > unit_tests.
>
>> a) aren't unit tests run on the ui thread?
>
> no, only if you explicitly create a MessageLoop of type UI. =A0normally
> there's no MessageLoop::current() on unit tests.
>
>> b) you actually changed the check from not io and not file to not ui
>
> and not io
>
> oops, I copied and pasted wrong. =A0fixed.
>
> http://codereview.chromium.org/342068
>

LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6