New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 815225 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 729596



Sign in to add a comment

Move process launching thread to child process launcher code

Project Member Reported by hanxi@chromium.org, Feb 23 2018

Issue description

Currently the process launching thread is in browser_main_loop. Since it is only used for launching a new process, it should be owned by child process launcher, and is lazy initialized upon a request to launch process.
 

Comment 1 by hanxi@chromium.org, Feb 23 2018

Labels: -Pri-3 Pri-2

Comment 2 by hanxi@chromium.org, Feb 23 2018

Blocking: 729596

Comment 3 by hanxi@chromium.org, Feb 28 2018

Owner: hanxi@chromium.org
Status: Started (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/89d93df220a35ef162d662c27cb3ea3d643c4fd9

commit 89d93df220a35ef162d662c27cb3ea3d643c4fd9
Author: Xi Han <hanxi@google.com>
Date: Fri Mar 09 20:55:07 2018

Move the TaskRunner for process launching to ChildProcessLauncherHelper.

ServiceManager may need to launch child processes before a BrowserMainLoop
has been created, and therefore before any BrowserThreads exists. We
therefore replace the BrowserThread::PROCESS_LAUNCHER with a global
TaskScheduler sequence, created on-demand when the ServiceManager or
ChildProcessLauncherHelper first need it.

Under Windows we must use a single-thread TaskRunner, while on other
platforms we are able to use a normal TaskScheduler sequence. File
crbug.com/820200 to track that.

Bug:  815225 
Change-Id: Ia0f46461fb9cc92fddacf81ee96b764de8477d11
Reviewed-on: https://chromium-review.googlesource.com/941264
Commit-Queue: Xi Han <hanxi@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542212}
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/android_webview/browser/aw_browser_terminator.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/chrome/browser/metrics/thread_watcher_report_hang.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/chrome/browser/service_process/service_process_control.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/browser_main_loop.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/browser_main_loop.h
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/browser_thread_impl.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/child_process_launcher.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/child_process_launcher_helper.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/child_process_launcher_helper_android.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/child_process_launcher_helper_fuchsia.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/child_process_launcher_helper_linux.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/child_process_launcher_helper_mac.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/child_process_launcher_helper_win.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/browser/renderer_host/render_process_host_browsertest.cc
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/public/android/java/src/org/chromium/content/browser/LauncherThread.java
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/public/browser/BUILD.gn
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/public/browser/browser_thread.h
[add] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/public/browser/child_process_launcher_utils.h
[modify] https://crrev.com/89d93df220a35ef162d662c27cb3ea3d643c4fd9/content/public/test/test_browser_thread_bundle.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2e84e1812b090efbebd4b22edaecac3aa036f4e8

commit 2e84e1812b090efbebd4b22edaecac3aa036f4e8
Author: Gabriel Charette <gab@chromium.org>
Date: Thu Mar 15 05:49:40 2018

More BrowserThread cleanups after removal of PROCESS_LAUNCHER thread.

Follow-up to https://chromium-review.googlesource.com/c/chromium/src/+/941264

Further cleanup of WithBaseSyncPrimitives() is done in
https://chromium-review.googlesource.com/c/chromium/src/+/961455
as that may have side-effects that risk a revert.

Bug:  815225 ,  689520 
Change-Id: I8ba8bb6709b7d8cc88e07e0f915c3c00d0a80cc2
Reviewed-on: https://chromium-review.googlesource.com/961451
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543311}
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/components/metrics/call_stack_profile_metrics_provider.cc
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/components/metrics/call_stack_profile_params.h
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/components/metrics/public/cpp/call_stack_profile_struct_traits.h
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/components/metrics/public/cpp/call_stack_profile_struct_traits_unittest.cc
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/components/metrics/public/interfaces/call_stack_profile_collector.mojom
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/content/browser/browser_main_loop.cc
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/content/browser/browser_main_loop.h
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/content/browser/browser_thread_impl.cc
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/content/browser/browser_thread_impl.h
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/content/browser/child_process_launcher_helper.cc
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/content/public/browser/browser_thread.h
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/docs/threading_and_tasks.md
[modify] https://crrev.com/2e84e1812b090efbebd4b22edaecac3aa036f4e8/third_party/metrics_proto/execution_context.proto

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c780f2fd580f23b9d2b25f9748bee73904519017

commit c780f2fd580f23b9d2b25f9748bee73904519017
Author: Gabriel Charette <gab@chromium.org>
Date: Thu Mar 15 13:08:21 2018

Do not allow sync primitives on the process launcher thread.

It was merely added to match previous allowance before
migration to TaskScheduler, but this allowance doesn't
appear to be needed (bots are happy without).

Bug:  815225 
Change-Id: Ie8ab830fe3807ed29fa91ea06c30b326082e93c6
Reviewed-on: https://chromium-review.googlesource.com/961455
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543355}
[modify] https://crrev.com/c780f2fd580f23b9d2b25f9748bee73904519017/content/browser/child_process_launcher_helper.cc

Comment 7 by hanxi@chromium.org, Mar 15 2018

Cc: gab@chromium.org
Thank you Cabriel for the further clean up!

Comment 8 by gab@chromium.org, Mar 15 2018

Np, one more coming down the pipe :) https://chromium-review.googlesource.com/c/chromium/src/+/963361. PROCESS_LAUMCHER was the very last string of a very long cleanup of BrowserThreads, glad we can finally remove all of these temporary APIs and guards :).

Having only UI/IO remaining will allow even further cleanup in  issue 821034  :).

PS: did you see my comment on your original CL? You should use LazySingleThreadTaskRunner instead of a NoDestructor<SingleThreadTaskRunner> because the former will guarantee recycling in unit tests that go through multiple ScopedTaskEnvironment on the same process.

Comment 9 by hanxi@chromium.org, Mar 15 2018

Oh, I missed that comment, will address it in a follow up CL:)

Yeah, we will wait for your CL landed to close this bug!
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d883944509f06376e6f40c217142333644cfdc7a

commit d883944509f06376e6f40c217142333644cfdc7a
Author: Gabriel Charette <gab@chromium.org>
Date: Thu Mar 15 18:56:22 2018

Cleanup base APIs after PROCESS_LAUNCHER removal.

Follow-up to https://chromium-review.googlesource.com/c/chromium/src/+/941264

Now that only BrowserThread::UI/IO remain: APIs used to prevent
further abuse in other BrowserThreads during their deprecation are no
longer needed :).

R=fdoray@chromium.org

Bug:  815225 
Change-Id: Ifa952367cd2ba87d035c14f0428393644503ef18
Reviewed-on: https://chromium-review.googlesource.com/963361
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543462}
[modify] https://crrev.com/d883944509f06376e6f40c217142333644cfdc7a/base/message_loop/message_loop.cc
[modify] https://crrev.com/d883944509f06376e6f40c217142333644cfdc7a/base/message_loop/message_loop.h
[modify] https://crrev.com/d883944509f06376e6f40c217142333644cfdc7a/base/run_loop.cc
[modify] https://crrev.com/d883944509f06376e6f40c217142333644cfdc7a/base/run_loop.h
[modify] https://crrev.com/d883944509f06376e6f40c217142333644cfdc7a/base/run_loop_unittest.cc
[modify] https://crrev.com/d883944509f06376e6f40c217142333644cfdc7a/mojo/public/cpp/bindings/lib/connector.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c4bbd5137696f1bea56a5a50e89ac3e69fa20247

commit c4bbd5137696f1bea56a5a50e89ac3e69fa20247
Author: Chris Hamilton <chrisha@chromium.org>
Date: Fri Mar 16 15:37:26 2018

Remove dangling process launcher thread reference.

This thread was recently removed, and is no longer used.

BUG= 815225 

Change-Id: I11e14933e9f30791be835034f59296bdbbedb708
Reviewed-on: https://chromium-review.googlesource.com/963597
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543708}
[modify] https://crrev.com/c4bbd5137696f1bea56a5a50e89ac3e69fa20247/content/public/test/test_browser_thread_bundle.h

Project Member

Comment 12 by bugdroid1@chromium.org, Mar 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9340143c4da294532be6d3e4892636cc417a674d

commit 9340143c4da294532be6d3e4892636cc417a674d
Author: Xi Han <hanxi@google.com>
Date: Fri Mar 16 20:50:49 2018

Use LazySingleThreadTaskRunner for process launcher thread.

This is a follow up to CL (https://chromium-review.googlesource.com/c/chromium/src/+/941264).
LazySingleThreadTaskRunner allows the created single-thread task runner be be
recycled in unittests.

Bug:  815225 
Change-Id: I70402798883f3550aced26ef07d48d8e882967f4
Reviewed-on: https://chromium-review.googlesource.com/964782
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543822}
[modify] https://crrev.com/9340143c4da294532be6d3e4892636cc417a674d/content/browser/child_process_launcher_helper.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Mar 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/790754cf4c27c9e1b33613c4e1a48c80c82172ba

commit 790754cf4c27c9e1b33613c4e1a48c80c82172ba
Author: Gabriel Charette <gab@chromium.org>
Date: Fri Mar 16 21:32:59 2018

Fix IWYU for BrowserThread::

Missing includes are blocking a bigger upcoming change that
removes browser_thread.h from other places which tend to have a far
reach.

This removal was done by a script which adds browser_thread.h if
the file has "BrowserThread::" on a line which isn't a comment.

If the file is a header it also substracts the include from matching .cc
file if any.

If the file is a .cc it omits the include if it's already in the
matching .h

Minor exceptions :
 * files that already included test_browser_thread.h (not doing so
   broke DEPS as some tests are explicitly allowed to include
   test_browser_thread.h but no content/public)
 * chrome/renderer/chrome_content_renderer_client_browsertest.cc
   (because it violates DEPS already by using BrowserThread:: from
    content/renderer but this CL doesn't aim to fix tricky use cases)

TBR=jam@chromium.org

Bug:  815225 ,  821034 
Change-Id: I5274433918c85f26caaa5b99adb990f07222fb99
Reviewed-on: https://chromium-review.googlesource.com/967186
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543834}
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/android_webview/browser/aw_printing_message_filter.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/android_webview/browser/aw_printing_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/apps/guest_view/web_view_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/browsing_data/browsing_data_local_storage_helper_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/browsing_data/counters/cache_counter_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/chromeos/accessibility/select_to_speak_event_rewriter.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/chromeos/arc/print/arc_print_service.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/chromeos/extensions/external_cache_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/chromeos/extensions/file_manager/private_api_file_system.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/chromeos/extensions/quick_unlock_private/quick_unlock_private_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/chromeos/extensions/wallpaper_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/chromeos/extensions/wallpaper_function_base.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/chromeos/login/login_utils_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/devtools/device/devtools_android_bridge.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/devtools/device/devtools_device_discovery.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/devtools/devtools_sanity_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/devtools/devtools_ui_bindings.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/download/chrome_download_manager_delegate.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/download/download_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/download/download_commands.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/download/download_crx_util.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/download/notification/download_notification_interactive_uitest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/bookmarks/bookmarks_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/content_settings/content_settings_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/data_reduction_proxy/data_reduction_proxy_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/declarative_content/chrome_content_rules_registry.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/developer_private/entry_picker.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/downloads/downloads_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/easy_unlock_private/easy_unlock_private_connection.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/enterprise_platform_keys/enterprise_platform_keys_apitest_nss.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/enterprise_platform_keys_private/enterprise_platform_keys_private_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/music_manager_private/music_manager_private_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/page_capture/page_capture_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/page_capture/page_capture_apitest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/platform_keys/platform_keys_test_base.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/processes/processes_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/streams_private/streams_private_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/sync_file_system/sync_file_system_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/blacklist.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/blacklist_state_fetcher.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/content_verifier_hash_fetch_behavior_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/extension_messages_apitest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/extensions/extension_service.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/icon_loader_auralinux.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/icon_loader_chromeos.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/lifetime/browser_close_manager_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/media/router/mojo/media_router_desktop.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/metrics/thread_watcher_android.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/net/predictor_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/notifications/notification_platform_bridge_mac.mm
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/password_manager/password_manager_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/password_manager/password_manager_test_base.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/pdf/pdf_extension_test.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/plugins/plugin_info_host_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/plugins/plugin_info_host_impl.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/predictors/loading_predictor.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/prerender/prerender_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/prerender/prerender_message_filter.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/prerender/prerender_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/prerender/prerender_nostate_prefetch_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/printing/printing_message_filter.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/printing/printing_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/renderer_context_menu/spelling_menu_observer.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/renderer_host/chrome_extension_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/renderer_host/chrome_render_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/renderer_host/site_per_process_text_input_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/safe_browsing/certificate_reporting_service_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/safe_browsing/chrome_password_protection_service.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/safe_browsing/download_protection/check_client_download_request.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/safe_browsing/download_protection/download_feedback_service.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/safe_browsing/download_protection/download_protection_service.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/safe_browsing/threat_details_unittest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/speech/tts_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/ssl/chrome_expect_ct_reporter_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/ssl/security_state_tab_helper_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/ssl/ssl_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/ui/cocoa/apps/quit_with_apps_controller_mac.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/ui/global_error/global_error_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/ui/views/location_bar/location_bar_view_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/ui/views/ssl_client_certificate_selector_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/ui/webui/media_router/media_router_dialog_controller_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/chrome/browser/ui/webui/net_internals/net_internals_ui_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/components/guest_view/browser/guest_view_message_filter.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/components/guest_view/browser/guest_view_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/accessibility/accessibility_ui.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/android/dialog_overlay_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/android/java/gin_java_bridge_message_filter.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/background_fetch/background_fetch_delegate_proxy_unittest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/background_fetch/background_fetch_scheduler_unittest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/background_sync/background_sync_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/blob_storage/blob_dispatcher_host.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/browser_main_loop.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/browser_main_loop_unittest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/browser_plugin/browser_plugin_message_filter.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/browser_plugin/browser_plugin_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/browser_process_sub_thread.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/browser_side_navigation_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/browsing_data/clear_site_data_throttle_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/cache_storage/cache_storage_dispatcher_host.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/compositor/gpu_process_transport_factory.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/devtools/devtools_interceptor_controller.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/devtools/devtools_target_registry.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/devtools/protocol/storage_handler.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/devtools/render_frame_devtools_agent_host.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/dom_storage/dom_storage_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/dom_storage/dom_storage_context_wrapper.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/dom_storage/dom_storage_message_filter.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/download/drag_download_file_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/download/mhtml_generation_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/fileapi/fileapi_message_filter.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/frame_host/navigation_handle_impl_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/frame_host/render_frame_message_filter_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/gpu/gpu_ipc_browsertests.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/gpu/in_process_gpu_thread_browsertests.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/indexed_db/database_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/indexed_db/indexed_db_callbacks.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/indexed_db/indexed_db_callbacks.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/loader/cross_site_document_blocking_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/loader/prefetch_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/media/android/browser_surface_view_manager.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/message_port_provider.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/network_service_client.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/network_service_restart_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/clipboard_host_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/media/audio_input_renderer_host.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/media/audio_renderer_host.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/media/peer_connection_tracker_host.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/media/video_capture_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/media/video_capture_host.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/pepper/pepper_renderer_connection.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/render_message_filter.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/render_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/render_process_host_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/render_view_host_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/renderer_host/text_input_client_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/security_exploit_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/site_per_process_hit_test_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/site_per_process_mac_browsertest.mm
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/speech/speech_recognition_dispatcher_host.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/speech/speech_recognition_dispatcher_host.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/speech/speech_recognizer_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/tracing/background_tracing_manager_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/web_contents/web_contents_impl_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/web_contents/web_contents_view_aura_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/browser/web_package/web_package_request_handler_browsertest.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/public/browser/browser_thread.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/public/test/content_browser_test_utils.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/public/test/fake_speech_recognition_manager.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/public/test/test_frame_navigation_observer.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/shell/browser/layout_test/layout_test_message_filter.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/content/test/content_browser_test_utils_internal.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/extensions/browser/api/bluetooth/bluetooth_event_router.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/extensions/browser/api/bluetooth_low_energy/bluetooth_api_advertisement.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_connection.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_notify_session.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/extensions/browser/api/bluetooth_socket/bluetooth_api_socket.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/extensions/browser/api/bluetooth_socket/bluetooth_socket_api.cc
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/extensions/browser/api/bluetooth_socket/bluetooth_socket_api.h
[modify] https://crrev.com/790754cf4c27c9e1b33613c4e1a48c80c82172ba/extensions/browser/api/bluetooth_socket/bluetooth_socket_event_dispat
Project Member

Comment 14 by bugdroid1@chromium.org, Mar 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1dfd20ea813216b96f64ffff895e37f355866c4b

commit 1dfd20ea813216b96f64ffff895e37f355866c4b
Author: Gabriel Charette <gab@chromium.org>
Date: Fri Mar 16 21:40:26 2018

Cleanup unmatched TRACE_EVENT_END in BrowserMainLoop::CreateThreads

Trace event left behind after matching TRACE_EVENT_BEGIN was removed in
https://chromium-review.googlesource.com/c/chromium/src/+/941264/26/content/browser/browser_main_loop.cc

R=jam@chromium.org

Bug:  815225 
Change-Id: I5c1e2d94429e1b6366662ab01cda614a7246a4b3
Reviewed-on: https://chromium-review.googlesource.com/966933
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543835}
[modify] https://crrev.com/1dfd20ea813216b96f64ffff895e37f355866c4b/content/browser/browser_main_loop.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da103185169afb9f5bfb0938b083943ede24b2f4

commit da103185169afb9f5bfb0938b083943ede24b2f4
Author: Gabriel Charette <gab@chromium.org>
Date: Sat Mar 17 02:45:00 2018

Remove unused includes of browser_thread_impl.h

Scripted cleanup for upcoming CL. Removes browser_thread_impl.h from
all files that didn't have "BrowserThreadImpl".

TBR=jam@chromium.org

Bug:  815225 
Change-Id: Ica66babda894ef3f43981227f48e84f659a57897
Reviewed-on: https://chromium-review.googlesource.com/967545
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543922}
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/background_sync/background_sync_manager_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/browser_plugin/browser_plugin_guest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/compositor/viz_process_transport_factory.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/indexed_db/indexed_db_dispatcher_host_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/indexed_db/indexed_db_quota_client_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/loader/resource_dispatcher_host_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/loader/resource_loader_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/message_port_provider.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/renderer_host/media/media_stream_manager_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/renderer_host/media/video_capture_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/service_worker/service_worker_context_request_handler_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/service_worker/service_worker_context_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/service_worker/service_worker_controllee_request_handler_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/service_worker/service_worker_job_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/service_worker/service_worker_storage_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/site_instance_impl_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/browser/storage_partition_impl_unittest.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/public/test/test_browser_thread_bundle.cc
[modify] https://crrev.com/da103185169afb9f5bfb0938b083943ede24b2f4/content/public/test/test_host_resolver.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d260e9cf660f062b203692601cf9b6ccb27f3d1e

commit d260e9cf660f062b203692601cf9b6ccb27f3d1e
Author: Gabriel Charette <gab@chromium.org>
Date: Tue Mar 20 18:10:45 2018

Refactor BrowserThreadImpl, BrowserProcessSubThread, and BrowserMainLoop

This brings back the invariant that BrowserThread::IO isn't available
before BrowserMainLoop::CreateThreads(). This was broken to fix issue
729596 to bring up the thread earlier for ServiceManager but it is
important that code that posts to BrowserThread::IO statically have an
happens-after relationship to BrowserMainLoop::CreateThreads(). Exposing
it statically earlier put that invariant at risk.

Thankfully fixing  issue 815225  resulted in finally reaching the long
sought goal of only having BrowserThread::UI/IO. Now that the IO thread
is also kicked off before it's named statically, BrowserThreadImpl no
longer needs to be a base::Thread, hence this refactoring.

Before this CL:
 * BrowserThreadImpl was a base::Thread
   (could be a fake thread if SetMessageLoop was used)
 * BrowserProcessSubThread was a BrowserThreadImpl
   (performed a bit more initialization)
 * BrowserProcessSubThread was only used in production (in
   BrowserMainLoop)
 * BrowserThreadImpl was used for fake threads (BrowserMainLoop for
   BrowserThread::UI) and for testing (TestBrowserThread(Impl)).
 * BrowserThreadImpl overrode Init/Run/CleanUp() from base::Thread to
   perform some sanity checks as well as drive IOThread's Delegate (ref.
   BrowserThread::SetIOThreadDelegate())
 * BrowserProcessSubThread re-overrode Init/Run/CleanUp() to perform
   per-thread //content initialization (tests missed out on that per
   TestBrowserThread bypassing BrowserProcessSubThread by directly
   subclassing BrowserThreadImpl).

With this CL:
 * BrowserThreadImpl is merely a scoped object that binds a provided
   SingleThreadTaskRunner to a BrowserThread::ID.
 * BrowserProcessSubThread is a base::Thread and performs all of the
   initialization and cleanup specific to //content (this means it now
   also manages BrowserThread::SetIOThreadDelegate())
 * BrowserProcessSubThread can be brought up early before being bound to
   a BrowserThread::ID (BrowserMainLoop handles that through
   BrowserProcessSubThread ::RegisterAsBrowserThread())

Unfortunate exceptions required for this CL:
 * IOThread::Init() (invoked through BrowserThreadDelegate) perfoms
   blocking operations this was previously performed before installed
   the ThreadRestrictions on BrowserThread::IO. But now that //content
   is initialized after bringing up the thread, a
   base::ScopedAllowBlocking is required in scope of IOThread::Init().
 * TestBrowserThread previously bypassing BrowserProcessSubThread by
   directly subclassing BrowserThreadImpl meant it wasn't subject to
   ThreadRestrictions (unfortunate becomes it denies allowance
   verification to product code running in unit tests). Adding it back
   causes DCHECKs, as such
   BrowserProcessSubThread::AllowBlockingForTesting was added to allow
   this CL to pass CQ.

Of note:
 * BrowserProcessSubThread is still written as though it supports many
   BrowserThread::IDs but in practice it's mostly always
   BrowserThread::IO (except in ThreadWatcherTest I think). This change
   was big enough that I didn't bother also breaking that
   generalization.
 * BrowserThreadImpl's constructor was made private to ensure only
   BrowserProcessSubThread and a few select callers get to drive it (to
   avoid previous missed initialization issues)
 * Atomics to manage BrowserThread::SetIOThreadDelegate were removed.
   Restriction was instead added that this only be called before
   initialization and after shutdown (this was already the case).

Follow-ups to this CL:
 * //ios duplicates this logic and will need to undergo the same change
   as a follow-up
 * Fixing ios will allow removal of base::Thread::SetMessageLoop hack :)
 * Removing BrowserThreadGlobals::lock_ to address  crbug.com/821034  will
   be much easier
 * BrowserThread post APIs should DCHECK rather than no-op if using a
   BrowserThread::ID before it's registered.

Bug:  815225 ,  821034 , 729596
Change-Id: If1038f23079df72203b1e95c7d26647f8824a726
Reviewed-on: https://chromium-review.googlesource.com/969104
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544440}
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/base/threading/thread.h
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/base/threading/thread_restrictions.h
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/chrome/browser/browser_process_impl_unittest.cc
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/browser/browser_main_loop.cc
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/browser/browser_main_loop.h
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/browser/browser_main_loop_unittest.cc
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/browser/browser_main_runner.cc
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/browser/browser_process_sub_thread.cc
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/browser/browser_process_sub_thread.h
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/browser/browser_thread_impl.cc
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/browser/browser_thread_impl.h
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/browser/browser_thread_unittest.cc
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/public/browser/browser_thread.h
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/public/browser/browser_thread_delegate.h
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/public/test/test_browser_thread.cc
[modify] https://crrev.com/d260e9cf660f062b203692601cf9b6ccb27f3d1e/content/public/test/test_browser_thread.h

Comment 17 by hanxi@chromium.org, Mar 20 2018

Status: Fixed (was: Started)
Mark it as fixed, thank you gab@!
Project Member

Comment 18 by bugdroid1@chromium.org, Mar 26 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fa239c8c41939589490f62c39321b8c2504b2219

commit fa239c8c41939589490f62c39321b8c2504b2219
Author: Matt Falkenhagen <falken@chromium.org>
Date: Mon Mar 26 04:21:19 2018

Revert "Refactor BrowserThreadImpl, BrowserProcessSubThread, and BrowserMainLoop"

This reverts commit d260e9cf660f062b203692601cf9b6ccb27f3d1e.

Reason for revert: In Windows Canary versions since 67.0.3377.0, where this commit landed, IO thread hang reports have spiked dramatically. It is now the #1 browser crash report on Windows Canary at 33% of reports. I'm speculatively reverting this to see if the crash rate heals.

Original change's description:
> Refactor BrowserThreadImpl, BrowserProcessSubThread, and BrowserMainLoop
>
> This brings back the invariant that BrowserThread::IO isn't available
> before BrowserMainLoop::CreateThreads(). This was broken to fix issue
> 729596 to bring up the thread earlier for ServiceManager but it is
> important that code that posts to BrowserThread::IO statically have an
> happens-after relationship to BrowserMainLoop::CreateThreads(). Exposing
> it statically earlier put that invariant at risk.
>
> Thankfully fixing  issue 815225  resulted in finally reaching the long
> sought goal of only having BrowserThread::UI/IO. Now that the IO thread
> is also kicked off before it's named statically, BrowserThreadImpl no
> longer needs to be a base::Thread, hence this refactoring.
>
> Before this CL:
>  * BrowserThreadImpl was a base::Thread
>    (could be a fake thread if SetMessageLoop was used)
>  * BrowserProcessSubThread was a BrowserThreadImpl
>    (performed a bit more initialization)
>  * BrowserProcessSubThread was only used in production (in
>    BrowserMainLoop)
>  * BrowserThreadImpl was used for fake threads (BrowserMainLoop for
>    BrowserThread::UI) and for testing (TestBrowserThread(Impl)).
>  * BrowserThreadImpl overrode Init/Run/CleanUp() from base::Thread to
>    perform some sanity checks as well as drive IOThread's Delegate (ref.
>    BrowserThread::SetIOThreadDelegate())
>  * BrowserProcessSubThread re-overrode Init/Run/CleanUp() to perform
>    per-thread //content initialization (tests missed out on that per
>    TestBrowserThread bypassing BrowserProcessSubThread by directly
>    subclassing BrowserThreadImpl).
>
> With this CL:
>  * BrowserThreadImpl is merely a scoped object that binds a provided
>    SingleThreadTaskRunner to a BrowserThread::ID.
>  * BrowserProcessSubThread is a base::Thread and performs all of the
>    initialization and cleanup specific to //content (this means it now
>    also manages BrowserThread::SetIOThreadDelegate())
>  * BrowserProcessSubThread can be brought up early before being bound to
>    a BrowserThread::ID (BrowserMainLoop handles that through
>    BrowserProcessSubThread ::RegisterAsBrowserThread())
>
> Unfortunate exceptions required for this CL:
>  * IOThread::Init() (invoked through BrowserThreadDelegate) perfoms
>    blocking operations this was previously performed before installed
>    the ThreadRestrictions on BrowserThread::IO. But now that //content
>    is initialized after bringing up the thread, a
>    base::ScopedAllowBlocking is required in scope of IOThread::Init().
>  * TestBrowserThread previously bypassing BrowserProcessSubThread by
>    directly subclassing BrowserThreadImpl meant it wasn't subject to
>    ThreadRestrictions (unfortunate becomes it denies allowance
>    verification to product code running in unit tests). Adding it back
>    causes DCHECKs, as such
>    BrowserProcessSubThread::AllowBlockingForTesting was added to allow
>    this CL to pass CQ.
>
> Of note:
>  * BrowserProcessSubThread is still written as though it supports many
>    BrowserThread::IDs but in practice it's mostly always
>    BrowserThread::IO (except in ThreadWatcherTest I think). This change
>    was big enough that I didn't bother also breaking that
>    generalization.
>  * BrowserThreadImpl's constructor was made private to ensure only
>    BrowserProcessSubThread and a few select callers get to drive it (to
>    avoid previous missed initialization issues)
>  * Atomics to manage BrowserThread::SetIOThreadDelegate were removed.
>    Restriction was instead added that this only be called before
>    initialization and after shutdown (this was already the case).
>
> Follow-ups to this CL:
>  * //ios duplicates this logic and will need to undergo the same change
>    as a follow-up
>  * Fixing ios will allow removal of base::Thread::SetMessageLoop hack :)
>  * Removing BrowserThreadGlobals::lock_ to address  crbug.com/821034  will
>    be much easier
>  * BrowserThread post APIs should DCHECK rather than no-op if using a
>    BrowserThread::ID before it's registered.
>
> Bug:  815225 ,  821034 , 729596
> Change-Id: If1038f23079df72203b1e95c7d26647f8824a726
> Reviewed-on: https://chromium-review.googlesource.com/969104
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#544440}

TBR=gab@chromium.org,jam@chromium.org
NOPRESUBMIT=true

# Not skipping CQ checks because original CL landed > 1 day ago.
# falken: Skipping presubmit to use deprecated ThreadResrictions::DisallowWaiting().

Bug:  815225 ,  821034 , 729596
Change-Id: I2be97c5d8183497c005ab397c871f625b034d850
Reviewed-on: https://chromium-review.googlesource.com/979752
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545725}
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/base/threading/thread.h
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/base/threading/thread_restrictions.h
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/chrome/browser/browser_process_impl_unittest.cc
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/browser/browser_main_loop.cc
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/browser/browser_main_loop.h
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/browser/browser_main_loop_unittest.cc
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/browser/browser_main_runner.cc
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/browser/browser_process_sub_thread.cc
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/browser/browser_process_sub_thread.h
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/browser/browser_thread_impl.cc
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/browser/browser_thread_impl.h
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/browser/browser_thread_unittest.cc
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/public/browser/browser_thread.h
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/public/browser/browser_thread_delegate.h
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/public/test/test_browser_thread.cc
[modify] https://crrev.com/fa239c8c41939589490f62c39321b8c2504b2219/content/public/test/test_browser_thread.h

Project Member

Comment 19 by bugdroid1@chromium.org, Mar 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8eb4dff9914d111cbce7cbc756b0c96030151762

commit 8eb4dff9914d111cbce7cbc756b0c96030151762
Author: Gabriel Charette <gab@chromium.org>
Date: Tue Mar 27 14:22:54 2018

Reland "Refactor BrowserThreadImpl, BrowserProcessSubThread, and BrowserMainLoop"

This is a reland of d260e9cf660f062b203692601cf9b6ccb27f3d1e

It was reverted because of crbug.com/824716, these weren't new crashes
but known crashes mislabeled as a fallout of this change.
http://cl/190471699 fixes the crash backend to not rely on
"content::BrowserThreadImpl::IOThreadRun" being in the signature.

Only diff in this CL is to use base::debug::Alias() in methods that we
don't want optimized (i.e. IOThreadRun) out as CHECK_GT was seen as
optimized out in some of the reported crashes (even though the same
pattern as before was used by this CL..?)

Original change's description:
> Refactor BrowserThreadImpl, BrowserProcessSubThread, and BrowserMainLoop
>
> This brings back the invariant that BrowserThread::IO isn't available
> before BrowserMainLoop::CreateThreads(). This was broken to fix issue
> 729596 to bring up the thread earlier for ServiceManager but it is
> important that code that posts to BrowserThread::IO statically have an
> happens-after relationship to BrowserMainLoop::CreateThreads(). Exposing
> it statically earlier put that invariant at risk.
>
> Thankfully fixing  issue 815225  resulted in finally reaching the long
> sought goal of only having BrowserThread::UI/IO. Now that the IO thread
> is also kicked off before it's named statically, BrowserThreadImpl no
> longer needs to be a base::Thread, hence this refactoring.
>
> Before this CL:
>  * BrowserThreadImpl was a base::Thread
>    (could be a fake thread if SetMessageLoop was used)
>  * BrowserProcessSubThread was a BrowserThreadImpl
>    (performed a bit more initialization)
>  * BrowserProcessSubThread was only used in production (in
>    BrowserMainLoop)
>  * BrowserThreadImpl was used for fake threads (BrowserMainLoop for
>    BrowserThread::UI) and for testing (TestBrowserThread(Impl)).
>  * BrowserThreadImpl overrode Init/Run/CleanUp() from base::Thread to
>    perform some sanity checks as well as drive IOThread's Delegate (ref.
>    BrowserThread::SetIOThreadDelegate())
>  * BrowserProcessSubThread re-overrode Init/Run/CleanUp() to perform
>    per-thread //content initialization (tests missed out on that per
>    TestBrowserThread bypassing BrowserProcessSubThread by directly
>    subclassing BrowserThreadImpl).
>
> With this CL:
>  * BrowserThreadImpl is merely a scoped object that binds a provided
>    SingleThreadTaskRunner to a BrowserThread::ID.
>  * BrowserProcessSubThread is a base::Thread and performs all of the
>    initialization and cleanup specific to //content (this means it now
>    also manages BrowserThread::SetIOThreadDelegate())
>  * BrowserProcessSubThread can be brought up early before being bound to
>    a BrowserThread::ID (BrowserMainLoop handles that through
>    BrowserProcessSubThread ::RegisterAsBrowserThread())
>
> Unfortunate exceptions required for this CL:
>  * IOThread::Init() (invoked through BrowserThreadDelegate) perfoms
>    blocking operations this was previously performed before installed
>    the ThreadRestrictions on BrowserThread::IO. But now that //content
>    is initialized after bringing up the thread, a
>    base::ScopedAllowBlocking is required in scope of IOThread::Init().
>  * TestBrowserThread previously bypassing BrowserProcessSubThread by
>    directly subclassing BrowserThreadImpl meant it wasn't subject to
>    ThreadRestrictions (unfortunate becomes it denies allowance
>    verification to product code running in unit tests). Adding it back
>    causes DCHECKs, as such
>    BrowserProcessSubThread::AllowBlockingForTesting was added to allow
>    this CL to pass CQ.
>
> Of note:
>  * BrowserProcessSubThread is still written as though it supports many
>    BrowserThread::IDs but in practice it's mostly always
>    BrowserThread::IO (except in ThreadWatcherTest I think). This change
>    was big enough that I didn't bother also breaking that
>    generalization.
>  * BrowserThreadImpl's constructor was made private to ensure only
>    BrowserProcessSubThread and a few select callers get to drive it (to
>    avoid previous missed initialization issues)
>  * Atomics to manage BrowserThread::SetIOThreadDelegate were removed.
>    Restriction was instead added that this only be called before
>    initialization and after shutdown (this was already the case).
>
> Follow-ups to this CL:
>  * //ios duplicates this logic and will need to undergo the same change
>    as a follow-up
>  * Fixing ios will allow removal of base::Thread::SetMessageLoop hack :)
>  * Removing BrowserThreadGlobals::lock_ to address  crbug.com/821034  will
>    be much easier
>  * BrowserThread post APIs should DCHECK rather than no-op if using a
>    BrowserThread::ID before it's registered.
>
> Bug:  815225 ,  821034 , 729596
> Change-Id: If1038f23079df72203b1e95c7d26647f8824a726
> Reviewed-on: https://chromium-review.googlesource.com/969104
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#544440}

TBR=jam@chromium.org

Bug:  815225 ,  821034 , 729596, 824716
Change-Id: I9a180975c69a008f8519d1d3d44663aa58a74a92
Reviewed-on: https://chromium-review.googlesource.com/980793
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546104}
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/base/threading/thread.h
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/base/threading/thread_restrictions.h
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/chrome/browser/browser_process_impl_unittest.cc
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/browser/browser_main_loop.cc
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/browser/browser_main_loop.h
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/browser/browser_main_loop_unittest.cc
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/browser/browser_main_runner.cc
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/browser/browser_process_sub_thread.cc
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/browser/browser_process_sub_thread.h
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/browser/browser_thread_impl.cc
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/browser/browser_thread_impl.h
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/browser/browser_thread_unittest.cc
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/public/browser/browser_thread.h
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/public/browser/browser_thread_delegate.h
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/public/test/test_browser_thread.cc
[modify] https://crrev.com/8eb4dff9914d111cbce7cbc756b0c96030151762/content/public/test/test_browser_thread.h

Sign in to add a comment