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

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:
4 years, 6 months ago by jam
Modified:
2 years, 11 months ago
Reviewers:
darin, jorlow
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, timsteele
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) Lint Patch
M chrome/browser/autocomplete/search_provider_unittest.cc View 1 2 3 3 chunks +8 lines, -3 lines 4 comments 0 errors Download
M chrome/browser/browser_main.cc View 1 2 3 4 chunks +9 lines, -7 lines 3 comments 0 errors Download
M chrome/browser/browser_process_impl.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments 0 errors Download
M chrome/browser/browsing_data_remover.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments 0 errors Download
M chrome/browser/chrome_plugin_browsing_context.cc View 1 2 3 1 chunk +1 line, -7 lines 0 comments 0 errors Download
M chrome/browser/chrome_plugin_host.cc View 1 2 3 1 chunk +4 lines, -7 lines 0 comments 0 errors Download
M chrome/browser/chrome_thread.h View 1 2 3 6 chunks +72 lines, -29 lines 6 comments 1 errors Download
M chrome/browser/chrome_thread.cc View 1 2 3 5 chunks +101 lines, -28 lines 3 comments 1 errors Download
D chrome/browser/chrome_thread_unittest.cc View 1 2 3 1 chunk +0 lines, -68 lines 4 comments 0 errors Download
M chrome/browser/chromeos/cros_network_library.cc View 1 2 3 3 chunks +17 lines, -17 lines 0 comments 0 errors Download
M chrome/browser/chromeos/cros_power_library.cc View 1 2 3 2 chunks +9 lines, -10 lines 0 comments 0 errors Download
M chrome/browser/dom_ui/chrome_url_data_manager.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments 0 errors Download
M chrome/browser/download/download_file.cc View 3 chunks +15 lines, -6 lines 2 comments 0 errors Download
M chrome/browser/download/download_manager.h View 3 chunks +6 lines, -10 lines 0 comments 1 errors Download
M chrome/browser/download/download_manager.cc View 3 chunks +21 lines, -35 lines 2 comments 1 errors Download
M chrome/browser/extensions/autoupdate_interceptor.cc View 1 2 3 1 chunk +3 lines, -4 lines 0 comments 0 errors Download
M chrome/browser/extensions/extension_disabled_infobar_delegate.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments 0 errors Download
M chrome/browser/extensions/extension_message_service.cc View 1 2 3 2 chunks +2 lines, -4 lines 0 comments 0 errors Download
M chrome/browser/extensions/extension_updater_unittest.cc View 1 2 3 7 chunks +25 lines, -9 lines 0 comments 0 errors Download
M chrome/browser/extensions/extensions_ui.cc View 1 2 3 2 chunks +1 line, -3 lines 0 comments 0 errors Download
M chrome/browser/extensions/file_reader.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments 0 errors Download
MM chrome/browser/extensions/pack_extension_job.h View 1 2 3 2 chunks +1 line, -3 lines 0 comments 0 errors Download
MM chrome/browser/extensions/pack_extension_job.cc View 1 2 3 1 chunk +6 lines, -5 lines 0 comments 0 errors Download
M chrome/browser/extensions/sandboxed_extension_unpacker.cc View 1 2 3 1 chunk +4 lines, -2 lines 0 comments 0 errors Download
M chrome/browser/extensions/user_script_listener_unittest.cc View 1 2 3 2 chunks +3 lines, -2 lines 0 comments 0 errors Download
M chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h View 1 2 3 3 chunks +6 lines, -11 lines 0 comments 0 errors Download
M chrome/browser/in_process_webkit/dom_storage_dispatcher_host.cc View 1 2 3 14 chunks +28 lines, -51 lines 1 comment 1 errors Download
M chrome/browser/in_process_webkit/webkit_context.cc View 1 2 3 2 chunks +15 lines, -16 lines 2 comments 0 errors Download
M chrome/browser/in_process_webkit/webkit_thread.h View 1 2 3 2 chunks +3 lines, -23 lines 0 comments 0 errors Download
M chrome/browser/in_process_webkit/webkit_thread.cc View 1 2 3 2 chunks +9 lines, -25 lines 1 comment 0 errors Download
M chrome/browser/memory_details.cc View 1 2 3 2 chunks +3 lines, -3 lines 0 comments 0 errors Download
MM chrome/browser/memory_details_linux.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments 0 errors Download
MM chrome/browser/memory_details_win.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments 0 errors Download
M chrome/browser/net/chrome_url_request_context.cc View 1 2 3 2 chunks +2 lines, -4 lines 0 comments 0 errors Download
M chrome/browser/net/sqlite_persistent_cookie_store.h View 1 2 3 2 chunks +1 line, -6 lines 0 comments 1 errors Download
M chrome/browser/net/sqlite_persistent_cookie_store.cc View 1 2 3 8 chunks +15 lines, -20 lines 0 comments 0 errors Download
M chrome/browser/net/test_url_fetcher_factory.cc View 1 2 3 1 chunk +0 lines, -2 lines 0 comments 0 errors Download
M chrome/browser/net/url_fetcher.h View 1 2 3 3 chunks +0 lines, -19 lines 0 comments 0 errors Download
M chrome/browser/net/url_fetcher.cc View 1 2 3 8 chunks +15 lines, -19 lines 0 comments 0 errors Download
M chrome/browser/net/url_fetcher_unittest.cc View 1 2 3 10 chunks +17 lines, -12 lines 0 comments 0 errors Download
M chrome/browser/notifications/notification_object_proxy.cc View 1 2 3 1 chunk +3 lines, -5 lines 0 comments 0 errors Download
M chrome/browser/plugin_process_host.cc View 1 2 3 2 chunks +4 lines, -5 lines 0 comments 0 errors Download
M chrome/browser/plugin_process_host_mac.cc View 1 2 3 2 chunks +4 lines, -2 lines 0 comments 0 errors Download
M chrome/browser/plugin_service.cc View 1 2 3 3 chunks +3 lines, -6 lines 0 comments 0 errors Download
M chrome/browser/power_save_blocker_common.cc View 2 3 1 chunk +2 lines, -3 lines 0 comments 0 errors Download
M chrome/browser/printing/print_job_worker.cc View 1 2 3 2 chunks +6 lines, -6 lines 0 comments 0 errors Download
M chrome/browser/process_singleton_linux.cc View 1 2 3 1 chunk +1 line, -2 lines 0 comments 0 errors Download
M chrome/browser/profile_manager.cc View 1 2 3 2 chunks +2 lines, -6 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/buffered_resource_handler.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments 0 errors Download
M chrome/browser/renderer_host/file_system_accessor.cc View 1 2 3 1 chunk +2 lines, -2 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/file_system_accessor_unittest.cc View 1 2 3 2 chunks +0 lines, -2 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/resource_dispatcher_host.cc View 1 2 3 1 chunk +0 lines, -3 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/resource_dispatcher_host_unittest.cc View 1 2 3 3 chunks +5 lines, -1 line 0 comments 0 errors Download
M chrome/browser/renderer_host/resource_message_filter.h View 1 chunk +0 lines, -2 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/resource_message_filter.cc View 1 2 3 2 chunks +4 lines, -14 lines 0 comments 0 errors Download
M chrome/browser/renderer_host/resource_message_filter_gtk.cc View 1 2 3 9 chunks +36 lines, -24 lines 0 comments 11 errors Download
M chrome/browser/spellchecker.cc View 1 2 3 2 chunks +5 lines, -4 lines 0 comments 0 errors Download
M chrome/browser/sync/glue/http_bridge.h View 1 2 3 5 chunks +4 lines, -13 lines 0 comments 1 errors Download
M chrome/browser/sync/glue/http_bridge.cc View 1 2 3 7 chunks +17 lines, -20 lines 0 comments 1 errors Download
M chrome/browser/sync/glue/http_bridge_unittest.cc View 1 2 3 6 chunks +9 lines, -9 lines 0 comments 1 errors Download
M chrome/browser/user_data_manager.cc View 1 2 3 3 chunks +4 lines, -7 lines 0 comments 0 errors Download
M chrome/browser/web_resource/web_resource_service.cc View 1 2 3 1 chunk +2 lines, -1 line 0 comments 0 errors Download
M chrome/chrome.gyp View 1 2 3 1 chunk +0 lines, -1 line 0 comments 0 errors Download
M chrome/common/child_process_host.cc View 1 2 3 2 chunks +3 lines, -5 lines 0 comments 0 errors Download
M chrome/common/chrome_plugin_unittest.cc View 1 2 3 3 chunks +4 lines, -1 line 0 comments 0 errors Download
Commit:

Messages

Total messages: 6
jam
4 years, 6 months ago #1
darin
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 ...
4 years, 6 months ago #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 ...
4 years, 6 months ago #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: > ...
4 years, 6 months ago #4
darin
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: > ...
4 years, 6 months ago #5
jorlow
4 years, 5 months ago #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 1280:2d3e6564b7b6