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

Issue 306032: Simplify threading in browser thread by making only ChromeThread deal with di... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 10 months ago by jam
Modified:
4 years, 3 months ago
CC:
chromium-reviews_googlegroups.com, brettw+cc_chromium.org, ncarter, ben+cc_chromium.org, John Grabowski, Erik does not do reviews, idana, jam, Aaron Boodman, pam+watch_chromium.org, Paweł Hajdan Jr., darin (slow to review), tim (not reviewing)
Visibility:
Public.

Description

Simplify threading in browser thread by making only ChromeThread deal with different thread lifetimes. The rest of the code doesn't get MessageLoop pointers since they're not thread-safe and instead just call PostTask on ChromeThread. If the target thread is not alive, then the task is simply deleted. In a followup change, I'll remove any remaining MessageLoop* caching. With this change, there's little to be gained by caching since no locks are involved if the target MessageLoop is guaranteed to outlive the current thread (inferred automatically by the order of the chrome_threads_ array). BUG=25354 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=30163

Patch Set 1 : '' #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : a few more simplifications #

Total comments: 28
Unified diffs Side-by-side diffs Delta from patch set Stats (+554 lines, -626 lines) Patch
M chrome/browser/autocomplete/search_provider_unittest.cc View 1 2 3 3 chunks +8 lines, -3 lines 4 comments Download
M chrome/browser/browser_main.cc View 1 2 3 4 chunks +9 lines, -7 lines 3 comments Download
M chrome/browser/browser_process_impl.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/browsing_data_remover.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/chrome_plugin_browsing_context.cc View 1 2 3 1 chunk +1 line, -7 lines 0 comments Download
M chrome/browser/chrome_plugin_host.cc View 1 2 3 1 chunk +4 lines, -7 lines 0 comments Download
M chrome/browser/chrome_thread.h View 1 2 3 6 chunks +72 lines, -29 lines 6 comments Download
M chrome/browser/chrome_thread.cc View 1 2 3 5 chunks +101 lines, -28 lines 3 comments Download
D chrome/browser/chrome_thread_unittest.cc View 1 2 3 1 chunk +0 lines, -68 lines 4 comments Download
M chrome/browser/chromeos/cros_network_library.cc View 1 2 3 3 chunks +17 lines, -17 lines 0 comments Download
M chrome/browser/chromeos/cros_power_library.cc View 1 2 3 2 chunks +9 lines, -10 lines 0 comments Download
M chrome/browser/dom_ui/chrome_url_data_manager.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/download/download_file.cc View 3 chunks +15 lines, -6 lines 2 comments Download
M chrome/browser/download/download_manager.h View 3 chunks +6 lines, -10 lines 0 comments Download
M chrome/browser/download/download_manager.cc View 3 chunks +21 lines, -35 lines 2 comments Download
M chrome/browser/extensions/autoupdate_interceptor.cc View 1 2 3 1 chunk +3 lines, -4 lines 0 comments Download
M chrome/browser/extensions/extension_disabled_infobar_delegate.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/extension_message_service.cc View 1 2 3 2 chunks +2 lines, -4 lines 0 comments Download
M chrome/browser/extensions/extension_updater_unittest.cc View 1 2 3 7 chunks +25 lines, -9 lines 0 comments Download
M chrome/browser/extensions/extensions_ui.cc View 1 2 3 2 chunks +1 line, -3 lines 0 comments Download
M chrome/browser/extensions/file_reader.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
MM chrome/browser/extensions/pack_extension_job.h View 1 2 3 2 chunks +1 line, -3 lines 0 comments Download
MM chrome/browser/extensions/pack_extension_job.cc View 1 2 3 1 chunk +6 lines, -5 lines 0 comments Download
M chrome/browser/extensions/sandboxed_extension_unpacker.cc View 1 2 3 1 chunk +4 lines, -2 lines 0 comments Download
M chrome/browser/extensions/user_script_listener_unittest.cc View 1 2 3 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h View 1 2 3 3 chunks +6 lines, -11 lines 0 comments Download
M chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc View 1 2 3 14 chunks +28 lines, -51 lines 1 comment Download
M chrome/browser/in_process_webkit/webkit_context.cc View 1 2 3 2 chunks +15 lines, -16 lines 2 comments Download
M chrome/browser/in_process_webkit/webkit_thread.h View 1 2 3 2 chunks +3 lines, -23 lines 0 comments Download
M chrome/browser/in_process_webkit/webkit_thread.cc View 1 2 3 2 chunks +9 lines, -25 lines 1 comment Download
M chrome/browser/memory_details.cc View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
MM chrome/browser/memory_details_linux.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
MM chrome/browser/memory_details_win.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/net/chrome_url_request_context.cc View 1 2 3 2 chunks +2 lines, -4 lines 0 comments Download
M chrome/browser/net/sqlite_persistent_cookie_store.h View 1 2 3 2 chunks +1 line, -6 lines 0 comments Download
M chrome/browser/net/sqlite_persistent_cookie_store.cc View 1 2 3 8 chunks +15 lines, -20 lines 0 comments Download
M chrome/browser/net/test_url_fetcher_factory.cc View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/net/url_fetcher.h View 1 2 3 3 chunks +0 lines, -19 lines 0 comments Download
M chrome/browser/net/url_fetcher.cc View 1 2 3 8 chunks +15 lines, -19 lines 0 comments Download
M chrome/browser/net/url_fetcher_unittest.cc View 1 2 3 10 chunks +17 lines, -12 lines 0 comments Download
M chrome/browser/notifications/notification_object_proxy.cc View 1 2 3 1 chunk +3 lines, -5 lines 0 comments Download
M chrome/browser/plugin_process_host.cc View 1 2 3 2 chunks +4 lines, -5 lines 0 comments Download
M chrome/browser/plugin_process_host_mac.cc View 1 2 3 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/plugin_service.cc View 1 2 3 3 chunks +3 lines, -6 lines 0 comments Download
M chrome/browser/power_save_blocker_common.cc View 2 3 1 chunk +2 lines, -3 lines 0 comments Download
M chrome/browser/printing/print_job_worker.cc View 1 2 3 2 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/process_singleton_linux.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M chrome/browser/profile_manager.cc View 1 2 3 2 chunks +2 lines, -6 lines 0 comments Download
M chrome/browser/renderer_host/buffered_resource_handler.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/renderer_host/file_system_accessor.cc View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/renderer_host/file_system_accessor_unittest.cc View 1 2 3 2 chunks +0 lines, -2 lines 0 comments Download
M chrome/browser/renderer_host/resource_dispatcher_host.cc View 1 2 3 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc View 1 2 3 3 chunks +5 lines, -1 line 0 comments Download
M chrome/browser/renderer_host/resource_message_filter.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/renderer_host/resource_message_filter.cc View 1 2 3 2 chunks +4 lines, -14 lines 0 comments Download
M chrome/browser/renderer_host/resource_message_filter_gtk.cc View 1 2 3 9 chunks +36 lines, -24 lines 0 comments Download
M chrome/browser/spellchecker.cc View 1 2 3 2 chunks +5 lines, -4 lines 0 comments Download
M chrome/browser/sync/glue/http_bridge.h View 1 2 3 5 chunks +4 lines, -13 lines 0 comments Download
M chrome/browser/sync/glue/http_bridge.cc View 1 2 3 7 chunks +17 lines, -20 lines 0 comments Download
M chrome/browser/sync/glue/http_bridge_unittest.cc View 1 2 3 6 chunks +9 lines, -9 lines 0 comments Download
M chrome/browser/user_data_manager.cc View 1 2 3 3 chunks +4 lines, -7 lines 0 comments Download
M chrome/browser/web_resource/web_resource_service.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M chrome/chrome.gyp View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/child_process_host.cc View 1 2 3 2 chunks +3 lines, -5 lines 0 comments Download
M chrome/common/chrome_plugin_unittest.cc View 1 2 3 3 chunks +4 lines, -1 line 0 comments Download
Project "None" does not have a commit queue.

Messages

Total messages: 6 (0 generated)
jam
5 years, 10 months ago (2009-10-26 18:05:11 UTC) #1
darin (slow to review)
LGTM... just some minor things: http://codereview.chromium.org/306032/diff/16001/16016 File chrome/browser/autocomplete/search_provider_unittest.cc (right): http://codereview.chromium.org/306032/diff/16001/16016#newcode35 Line 35: io_thread_(ChromeThread::IO), does this ...
5 years, 10 months ago (2009-10-27 00:06:51 UTC) #2
jam
http://codereview.chromium.org/306032/diff/16001/16016 File chrome/browser/autocomplete/search_provider_unittest.cc (right): http://codereview.chromium.org/306032/diff/16001/16016#newcode35 Line 35: io_thread_(ChromeThread::IO), On 2009/10/27 00:06:52, darin wrote: > does ...
5 years, 10 months ago (2009-10-27 02:38:14 UTC) #3
jam
http://codereview.chromium.org/306032/diff/16001/16016 File chrome/browser/autocomplete/search_provider_unittest.cc (right): http://codereview.chromium.org/306032/diff/16001/16016#newcode35 Line 35: io_thread_(ChromeThread::IO), On 2009/10/27 02:38:18, John Abd-El-Malek wrote: > ...
5 years, 10 months ago (2009-10-27 02:46:29 UTC) #4
darin (slow to review)
http://codereview.chromium.org/306032/diff/16001/16016 File chrome/browser/autocomplete/search_provider_unittest.cc (right): http://codereview.chromium.org/306032/diff/16001/16016#newcode35 Line 35: io_thread_(ChromeThread::IO), On 2009/10/27 02:46:29, John Abd-El-Malek wrote: > ...
5 years, 10 months ago (2009-10-27 04:43:32 UTC) #5
jorlow
5 years, 10 months ago (2009-10-27 21:37:25 UTC) #6
http://codereview.chromium.org/306032/diff/16001/16026
File chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc (right):

http://codereview.chromium.org/306032/diff/16001/16026#newcode350
Line 350: ChromeThread::PostTask(ChromeThread::WEBKIT, FROM_HERE, task);
use from_here, not FROM_HERE

http://codereview.chromium.org/306032/diff/16001/16028
File chrome/browser/in_process_webkit/webkit_thread.cc (left):

http://codereview.chromium.org/306032/diff/16001/16028#oldcode22
Line 22: DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::IO));
Why did you remove this?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld c33a7a4