New issue
Advanced search Search tips

Issue 653916 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug


Sign in to add a comment

Redirect BrowserThreads to TaskScheduler

Project Member Reported by fdoray@chromium.org, Oct 7 2016

Issue description

BrowserThreads (except UI and IO) must be redirected to TaskScheduler.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 10 2016

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

commit f854c913531ac6ed65459c367070aad07179d531
Author: fdoray <fdoray@chromium.org>
Date: Mon Oct 10 14:51:07 2016

Run BrowserThreadTest.RunsTasksOnCurrentThreadDuringShutdown on UI thread.

In preparation for the migration of the FILE thread to TaskScheduler,
we want to disable destruction observers on it.
BrowserThreadTest.RunsTasksOnCurrentThreadDuringShutdown is the only
test that fails when destruction observers are disabled on the FILE
thread. This CL changes this test to use the UI thread instead.

BUG= 653916 

Review-Url: https://codereview.chromium.org/2399413003
Cr-Commit-Position: refs/heads/master@{#424151}

[modify] https://crrev.com/f854c913531ac6ed65459c367070aad07179d531/content/browser/browser_thread_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 11 2016

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

commit 8b65c3e37ffd93e7402d22cd053419a8a8edd862
Author: fdoray <fdoray@chromium.org>
Date: Tue Oct 11 20:37:31 2016

Do not run a nested loop in content::DeferredQuitRunLoop.

In preparation for the migration of BrowserThreads to
base/task_scheduler, we want to disallow running nested loops
on BrowserThreads (except IO and UI threads).
content::DeferredQuitRunLoop is currently the only code that
runs nested loops on these threads.

BUG= 653916 

Review-Url: https://codereview.chromium.org/2402553005
Cr-Commit-Position: refs/heads/master@{#424534}

[modify] https://crrev.com/8b65c3e37ffd93e7402d22cd053419a8a8edd862/chrome/browser/sync/test/integration/bookmarks_helper.cc
[modify] https://crrev.com/8b65c3e37ffd93e7402d22cd053419a8a8edd862/chrome/browser/ui/views/sync/one_click_signin_dialog_view_unittest.cc
[modify] https://crrev.com/8b65c3e37ffd93e7402d22cd053419a8a8edd862/content/public/test/test_utils.cc
[modify] https://crrev.com/8b65c3e37ffd93e7402d22cd053419a8a8edd862/content/public/test/test_utils.h

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 12 2016

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

commit 6d547b63b4176785a3caf5019c055173a28535e8
Author: fdoray <fdoray@chromium.org>
Date: Wed Oct 12 15:52:34 2016

Disallow nesting on some BrowserThreads.

This CL disallows nesting on BrowserThreads that will be migrated
to TaskScheduler. Running a nested loop or calling
Add/RemoveNestingObserver on these threads will result in a crash.

To our knowledge, no code runs a nested loop on BrowserThreads that
will be migrated to TaskScheduler. This CL will help us confirm that
in the wild.

BUG= 653916 

Review-Url: https://codereview.chromium.org/2407313002
Cr-Commit-Position: refs/heads/master@{#424749}

[modify] https://crrev.com/6d547b63b4176785a3caf5019c055173a28535e8/base/message_loop/message_loop.cc
[modify] https://crrev.com/6d547b63b4176785a3caf5019c055173a28535e8/base/message_loop/message_loop.h
[modify] https://crrev.com/6d547b63b4176785a3caf5019c055173a28535e8/content/browser/browser_thread_impl.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 14 2016

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

commit dbed8d7df17c48f391fde23becf4cb926079610a
Author: fdoray <fdoray@chromium.org>
Date: Fri Oct 14 19:12:38 2016

Disallow task observers on some BrowserThreads.

This CL disallows task observers on the DB, FILE, FILE_USER_BLOCKING,
PROCESS_LAUNCHER and CACHE threads. Calling Add/RemoveTaskObserver
on these threads will result in a crash.

This is important because these threads will be migrated to
TaskScheduler which doesn't support task observers.

To our knowledge, no code uses task observers on these threads. This
CL will help us confirm that in the wild.

BUG= 653916 

Review-Url: https://codereview.chromium.org/2419803002
Cr-Commit-Position: refs/heads/master@{#425418}

[modify] https://crrev.com/dbed8d7df17c48f391fde23becf4cb926079610a/base/message_loop/message_loop.cc
[modify] https://crrev.com/dbed8d7df17c48f391fde23becf4cb926079610a/base/message_loop/message_loop.h
[modify] https://crrev.com/dbed8d7df17c48f391fde23becf4cb926079610a/content/browser/browser_thread_impl.cc

Comment 5 by gab@chromium.org, Oct 18 2016

Labels: OS-All
Status: Assigned (was: Untriaged)

Comment 6 by fdoray@chromium.org, Oct 25 2016

Blockedon: 650723

Comment 7 by fdoray@chromium.org, Oct 25 2016

Blockedon: 645114

Comment 8 by fdoray@chromium.org, Oct 25 2016

Blockedon: 650318

Comment 11 by gab@chromium.org, Nov 1 2016

Blockedon: 661344
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 2 2016

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

commit 10ae436fe808a33451641e3b92e10f20954e28dc
Author: gab <gab@chromium.org>
Date: Wed Nov 02 23:34:53 2016

Remove dependency of sequenced_worker_pool.h on SingleThreadTaskRunner.

Breaks a cyclic dependency in an upcoming CL of mine.

Removing the include of single_thread_task_runner.h here breaks IWYU
all over the place!

The trickiest IWYU thing being exposed by this is that in order for
scoped_refptr<Foo> to be instantiated (not even used), Foo needs to be
fully defined.

This means that, by IWYU, anyone taking a scoped_refptr<Foo> by value or
returning one by value needs to #include "foo.h".

It's still fine to fwd-decl foo when passing via const scoped_refptr<Foo>&
or having a scoped_refptr<Foo> foo_; member in a class constructed/destroyed
out-of-line.

BUG= 653916 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
TBR=reviewers for IWUU side-effects (and minor other touchups):
  kmarshall@: blimp/
  danakj@: cc/
           storage/
  bartfab@: chrome/browser/chromeos/policy/
  gene@: chrome/service/
  maxbogue@: components/browser_sync/
             components/sync/
  bengr@: components/data_reduction_proxy/
  fukino@: components/drive/
  dimich@: components/gcm_driver/
  bradnelson@: components/nacl/
  achuith@: components/pairing/
  rsesek@: components/upload_list/
  mef@: components/wifi/
  michaeln@: content/browser/cache_storage/
             content/child/fileapi/
  dgozman@: content/browser/devtools/
  csharrison@: content/browser/loader/
  hbos@: content/renderer/media/
  mkwst@: content/shell/
  pfeldman@: device/usb/
  kbr@: gpu/
  droger@: ios/
  rockot@: ipc/
  dalecurtis@: media/
  agl@: net/
  garykac@: remoting/
  erg@: services/ui/
  dglazkov@: third_party/WebKit/Source/platform/

Review-Url: https://codereview.chromium.org/2443103003
Cr-Commit-Position: refs/heads/master@{#429451}

[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/base/threading/sequenced_worker_pool.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/base/threading/sequenced_worker_pool.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/blimp/client/core/context/assignment_fetcher.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/blimp/client/core/session/assignment_source.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/blimp/client/core/session/assignment_source.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/blimp/engine/renderer/frame_scheduler.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/cc/blimp/remote_compositor_bridge.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/cc/blimp/remote_compositor_bridge.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/cc/blimp/remote_compositor_bridge_client.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/cc/output/context_cache_controller.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/cc/output/context_cache_controller.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/cc/test/fake_remote_compositor_bridge.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/cc/test/fake_remote_compositor_bridge.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/chrome/browser/chromeos/policy/upload_job_impl.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/chrome/browser/chromeos/policy/upload_job_impl.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/chrome/service/service_utility_process_host.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/chrome/service/service_utility_process_host.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/browser_sync/signin_confirmation_helper.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/browser_sync/signin_confirmation_helper.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/drive/chromeos/file_system.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/drive/chromeos/file_system.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/drive/chromeos/file_system/get_file_for_saving_operation.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/drive/chromeos/file_system/get_file_for_saving_operation.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/drive/chromeos/resource_metadata.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/drive/file_system/get_file_for_saving_operation_unittest.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/drive/file_write_watcher.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/gcm_driver/crypto/gcm_key_store.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/gcm_driver/fake_gcm_driver.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/gcm_driver/fake_gcm_driver.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/nacl/common/nacl_debug_exception_handler_win.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/pairing/bluetooth_host_pairing_controller.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/pairing/bluetooth_host_pairing_controller.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/sync/model_impl/model_type_store_impl.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/sync/model_impl/model_type_store_impl.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/upload_list/crash_upload_list.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/components/wifi/wifi_service.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/content/browser/cache_storage/cache_storage_operation.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/content/browser/devtools/protocol/tethering_handler.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/content/browser/loader/upload_data_stream_builder.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/content/child/fileapi/webfilewriter_impl.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/content/public/browser/browser_thread.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/content/renderer/media/gpu/rtc_video_encoder.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/content/renderer/media/html_video_element_capturer_source.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/content/shell/browser/shell_url_request_context_getter.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/device/usb/usb_device_handle_android.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/device/usb/usb_device_handle_usbfs.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/device/usb/usb_service.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/gpu/ipc/client/command_buffer_proxy_impl.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/ios/web/public/web_thread.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/ipc/attachment_broker.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/ipc/attachment_broker_privileged.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/media/base/android/media_drm_bridge.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/media/base/media_url_demuxer.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/media/base/media_url_demuxer.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/media/renderers/video_overlay_factory.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/net/base/upload_file_element_reader.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/net/nqe/socket_watcher_factory.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/net/ssl/ssl_platform_key.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/net/ssl/ssl_platform_key_chromecast.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/net/ssl/ssl_platform_key_mac.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/net/ssl/ssl_platform_key_nss.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/net/ssl/ssl_platform_key_util.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/net/ssl/ssl_platform_key_win.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/client/queued_task_poster.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/client/queued_task_poster_unittest.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/host/fake_desktop_environment.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/host/fake_desktop_environment.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/host/native_messaging/log_message_handler.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/host/native_messaging/log_message_handler.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/protocol/ice_connection_to_client.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/protocol/ice_connection_to_client.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/protocol/ice_connection_to_host.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/protocol/webrtc_audio_source_adapter.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/protocol/webrtc_audio_source_adapter.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/protocol/webrtc_dummy_video_encoder.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/protocol/webrtc_video_stream.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/remoting/protocol/webrtc_video_stream.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/services/ui/public/cpp/gles2_context.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/services/ui/surfaces/surfaces_context_provider.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/services/ui/surfaces/surfaces_context_provider.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/services/ui/ws/server_window_compositor_frame_sink.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/services/ui/ws/server_window_compositor_frame_sink.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/storage/browser/blob/blob_async_builder_host.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/storage/browser/blob/blob_data_handle.cc
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/storage/browser/blob/blob_data_handle.h
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/third_party/WebKit/Source/platform/DEPS
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/third_party/WebKit/Source/platform/WebTaskRunner.cpp
[modify] https://crrev.com/10ae436fe808a33451641e3b92e10f20954e28dc/third_party/WebKit/Source/platform/heap/PersistentNode.cpp

Comment 13 by gab@chromium.org, Nov 3 2016

Blockedon: 662015

Comment 14 by gab@chromium.org, Nov 3 2016

Blockedon: 662055

Comment 15 by gab@chromium.org, Nov 3 2016

Blockedon: 662122

Comment 16 by gab@chromium.org, Nov 3 2016

Blockedon: 662191

Comment 17 by gab@chromium.org, Nov 7 2016

Components: Internals>TaskScheduler
Project Member

Comment 18 by bugdroid1@chromium.org, Nov 9 2016

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

commit f7f153ec850decdcc9bbd2f874c7b8e219bb321d
Author: gab <gab@chromium.org>
Date: Wed Nov 09 17:29:36 2016

Expose Thread::using_external_message_loop() to subclasses.

Extracted from https://codereview.chromium.org/2464233002/#ps260001 as I
flip flop back and forth between it and precursor CLs and having a change
in thread.h forces painful rebuilds...

BUG= 653916 

Review-Url: https://codereview.chromium.org/2484283005
Cr-Commit-Position: refs/heads/master@{#430960}

[modify] https://crrev.com/f7f153ec850decdcc9bbd2f874c7b8e219bb321d/base/threading/thread.h
[modify] https://crrev.com/f7f153ec850decdcc9bbd2f874c7b8e219bb321d/base/threading/thread_unittest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Nov 9 2016

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

commit 606d46cc6d881eb8553c1d7755458913e0e1af73
Author: gab <gab@chromium.org>
Date: Wed Nov 09 23:07:21 2016

Remove direct usage of BrowserThreadImpl in tests

Instantiating a BrowserThreadImpl sets global state which leaks to the next
tests. This is a problem in https://codereview.chromium.org/2464233002/ where
a reset was introduced for tests (driven by TestBrowserThread).

Tests should already have been using TestBrowserThread over BrowserThreadImpl
and today TestBrowserThreadBundle is favored over TestBrowserThread so it is
used as the replacement in this CL.

BUG= 653916 

Review-Url: https://codereview.chromium.org/2487073005
Cr-Commit-Position: refs/heads/master@{#431070}

[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/appcache/chrome_appcache_service_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/browser_thread_impl.h
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/download/base_file_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/indexed_db/indexed_db_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/loader/resource_scheduler_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/media/capture/audio_mirroring_manager_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/media/capture/desktop_capture_device_aura_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/renderer_host/media/media_stream_ui_controller_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/renderer_host/media/video_capture_manager_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/content/browser/resolve_proxy_msg_helper_unittest.cc
[modify] https://crrev.com/606d46cc6d881eb8553c1d7755458913e0e1af73/net/url_request/test_url_fetcher_factory.h

Comment 20 by gab@chromium.org, Nov 10 2016

Blockedon: 587199

Comment 21 by gab@chromium.org, Nov 10 2016

Blockedon: -587199

Comment 22 by gab@chromium.org, Nov 10 2016

Blockedon: 552633
Project Member

Comment 23 by bugdroid1@chromium.org, Nov 23 2016

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

commit 57d179c25000b2764d97096a7bfa910e9ffba89a
Author: gab <gab@chromium.org>
Date: Wed Nov 23 01:13:05 2016

TestBrowserThreads must outlive TestProfiles and TestProfileManagers

Otherwise they trigger an unhappy DCHECK that verifies that profiles
are destroyed on the UI thread.

This is a pre-requirement for https://codereview.chromium.org/2464233002
which will reset global BrowserThread state in ~BrowserThreadImpl()

The incorrect destruction order previously "worked" because BrowserThread
globals weren't cleaned up in between tests...

BUG= 653916 

Review-Url: https://codereview.chromium.org/2523093002
Cr-Commit-Position: refs/heads/master@{#434061}

[modify] https://crrev.com/57d179c25000b2764d97096a7bfa910e9ffba89a/chrome/browser/download/download_request_limiter_unittest.cc
[modify] https://crrev.com/57d179c25000b2764d97096a7bfa910e9ffba89a/chrome/browser/prerender/prerender_unittest.cc
[modify] https://crrev.com/57d179c25000b2764d97096a7bfa910e9ffba89a/chrome/browser/profiles/profile_info_cache_unittest.h
[modify] https://crrev.com/57d179c25000b2764d97096a7bfa910e9ffba89a/chrome/browser/profiles/profile_list_desktop_unittest.cc
[modify] https://crrev.com/57d179c25000b2764d97096a7bfa910e9ffba89a/chrome/browser/profiles/profile_statistics_unittest.cc
[modify] https://crrev.com/57d179c25000b2764d97096a7bfa910e9ffba89a/chrome/browser/sessions/session_restore_stats_collector_unittest.cc
[modify] https://crrev.com/57d179c25000b2764d97096a7bfa910e9ffba89a/chrome/browser/sessions/tab_loader_unittest.cc
[modify] https://crrev.com/57d179c25000b2764d97096a7bfa910e9ffba89a/chrome/browser/signin/chrome_signin_client_unittest.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Nov 23 2016

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

commit ca6c3690e7a1daaa6b428958dbf247c6611bb7a3
Author: gab <gab@chromium.org>
Date: Wed Nov 23 04:25:31 2016

Fix Thread::SetMessageLoop(nullptr).

Unit tests that pass base::MessageLoop::current() to a Thread need to
explicitly have a base::MessageLoop member as well or current() is null.

A better alternative though is to just have a content::TestBrowserThreadBundle member.

This long-standing issue (629139) was now blocking https://codereview.chromium.org/2464233002
which needs to unconditionally inspect the MessageLoop's member and it being unexpectedly null
is a problem.

Fixing the safe_browsing tests was a bit tricky because the TestSimpleTaskRunner they relied
on doesn't honor delays. Mocking the MessageLoop's TaskRunner with a TestMockTimeTaskRunner
was required. This in turn enabled better tests (that can explicitly wait for the expected timer
to fire). Ideally, their Timers would use the same MockTickClock but FastForwardUntilNoTasksRemain()
being the logical equivalent of the existing TestSimpleTaskRunner::RunUntilIdle() it was used instead
where required to simplify this CL.

BUG=629139,  653916 
TBR=danakj (re-enabling DCHECK in thread.cc)

Review-Url: https://codereview.chromium.org/2487343005
Cr-Commit-Position: refs/heads/master@{#434114}

[modify] https://crrev.com/ca6c3690e7a1daaa6b428958dbf247c6611bb7a3/base/threading/thread.cc
[modify] https://crrev.com/ca6c3690e7a1daaa6b428958dbf247c6611bb7a3/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc
[modify] https://crrev.com/ca6c3690e7a1daaa6b428958dbf247c6611bb7a3/chrome/browser/safe_browsing/protocol_manager.cc
[modify] https://crrev.com/ca6c3690e7a1daaa6b428958dbf247c6611bb7a3/chrome/browser/safe_browsing/protocol_manager.h
[modify] https://crrev.com/ca6c3690e7a1daaa6b428958dbf247c6611bb7a3/chrome/browser/safe_browsing/protocol_manager_unittest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Nov 29 2016

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

commit e65b9d238a74d05a7b496d2cee9e4bcbde7623ad
Author: gab <gab@chromium.org>
Date: Tue Nov 29 19:46:36 2016

Fix MediaDevices* tests dependency on BrowserThread globals after ~BrowserThreadImpl().

Mostly a matter of tweaking destruction order in test and adding an
additional flush instead of depending on ~MediaStreamManager() implicitly
flushing MediaStreamManager::video_capture_thread_().

This is a prereq to https://codereview.chromium.org/2464233002 which
will from now on reset BrowserThread globals associated with destroyed
BrowserThreadImpls and thus makes DCHECK_CURRENTLY_ON fail after that
fact.

BUG= 653916 

Review-Url: https://codereview.chromium.org/2535043003
Cr-Commit-Position: refs/heads/master@{#435055}

[modify] https://crrev.com/e65b9d238a74d05a7b496d2cee9e4bcbde7623ad/content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc
[modify] https://crrev.com/e65b9d238a74d05a7b496d2cee9e4bcbde7623ad/content/browser/renderer_host/media/media_devices_manager_unittest.cc
[modify] https://crrev.com/e65b9d238a74d05a7b496d2cee9e4bcbde7623ad/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/e65b9d238a74d05a7b496d2cee9e4bcbde7623ad/content/browser/renderer_host/media/media_stream_manager.h

Project Member

Comment 26 by bugdroid1@chromium.org, Dec 2 2016

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

commit 683111e5efbf225875eadf59804494611e9257d9
Author: gab <gab@chromium.org>
Date: Fri Dec 02 19:38:24 2016

Fix TestBrowserThread destruction order in Android history tests.

This is a prereq to https://codereview.chromium.org/2464233002 which
will from now on reset BrowserThread globals associated with destroyed
BrowserThreadImpls and thus makes DCHECK_CURRENTLY_ON fail after it
was.

BUG= 653916 

Review-Url: https://codereview.chromium.org/2543023003
Cr-Commit-Position: refs/heads/master@{#435995}

[modify] https://crrev.com/683111e5efbf225875eadf59804494611e9257d9/chrome/browser/history/android/android_provider_backend_unittest.cc
[modify] https://crrev.com/683111e5efbf225875eadf59804494611e9257d9/chrome/browser/history/android/bookmark_model_sql_handler_unittest.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Dec 3 2016

Project Member

Comment 28 by bugdroid1@chromium.org, Dec 8 2016

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

commit 90becc91928a53cd44e37dde76e3f29a9507f271
Author: gab <gab@chromium.org>
Date: Thu Dec 08 17:19:34 2016

Explicitly make BrowserThread::SetDelegate specific to the IO thread.

The IO thread was already the only user and officially restricting users
of this API is required as BrowserThreadDelegates won't be compatible
with threads redirected to the TaskScheduler.

BUG= 653916 

Review-Url: https://codereview.chromium.org/2558943002
Cr-Commit-Position: refs/heads/master@{#437271}

[modify] https://crrev.com/90becc91928a53cd44e37dde76e3f29a9507f271/chrome/browser/io_thread.cc
[modify] https://crrev.com/90becc91928a53cd44e37dde76e3f29a9507f271/content/browser/browser_thread_impl.cc
[modify] https://crrev.com/90becc91928a53cd44e37dde76e3f29a9507f271/content/public/browser/browser_thread.h
[modify] https://crrev.com/90becc91928a53cd44e37dde76e3f29a9507f271/content/public/browser/browser_thread_delegate.h

Project Member

Comment 29 by bugdroid1@chromium.org, Dec 12 2016

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

commit 9361390bd8a415b79cba242e70149bf6ca9a80ab
Author: gab <gab@chromium.org>
Date: Mon Dec 12 18:03:52 2016

Fix TestBrowserThreadBundle destruction order in IndexedDBDatabaseOperationTest.

IndexedDBDatabase requires BrowserThread::IO to be alive when it is
destroyed or it leaks some of its DeleteOnIOThread scoped_refptr members.
Not doing so results in LSAN failures in
https://codereview.chromium.org/2464233002 per it tightening
TestBrowserThread's shutdown semantics.

BUG= 653916 

Review-Url: https://codereview.chromium.org/2566693004
Cr-Commit-Position: refs/heads/master@{#437899}

[modify] https://crrev.com/9361390bd8a415b79cba242e70149bf6ca9a80ab/content/browser/indexed_db/indexed_db_database_unittest.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Dec 16 2016

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

commit 3ee0e445ae07397a077eb3213fc46318af01b43e
Author: gab <gab@chromium.org>
Date: Fri Dec 16 17:43:11 2016

Experiment with redirecting all BrowserThreads (but UI/IO) to TaskScheduler

This is part of the migration phase for TaskScheduler, design doc:
https://docs.google.com/document/d/1S2AAeoo1xa_vsLbDYBsDHCqhrkfiMgoIPlyRi6kxa5k/edit

Also generalizes BrowserThreadImpl away from the underlying thread's MessageLoop
and id in favor of TaskRunners and BrowserThreadState.

Cool side-effect: BrowserThreadImpl::StartWithOptions() no longer blocks on the
underlying thread starting (it used to via GetThreadId()), it now takes a reference
to its TaskRunner and lets the thread itself start on its own schedule :).
This wasn't previously intended to block (BrowserThreadImpl::StartAndWaitForTesting() is
supposed to be used for that) but had unintentionally regressed in http://crrev.com/408295
because GetThreadId() implicitly waits ( http://crbug.com/672977 ).

Another nice improvement is that ~BrowserThreadImpl() now cleans the BrowserThreadGlobals
associated with it, this has the side-effect to force proper destruction order of
TestBrowserThread in tests which brought forth a few precursor cleanups
( https://crbug.com/653916#c24 ) which will from now on be enforced by this CL.

When redirection is disabled, the logic should be the exact same as before.
 - Threads are brought up.
 - Tasks are posted to their MessageLoop (albeit through their TaskRunner now).
 - On shutdown, threads are joined one by one.

When redirection is enabled, we try to mimic this logic.
 - Redirection TaskRunners are only live (|accepting_tasks|) for the same period that the
   matching thread would be.
 - On shutdown, we block on each TaskRunner's queue being flushed one-by-one
   in the same order as in the no-redirection case (this almost identical to
   real threads join % one slight difference documented in detail in
   BrowserThreadImpl::StopRedirectionOfThreadID()).

BUG= 653916 ,  672977 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_tsan_rel_ng

TEST=
 A) "out\Release\chrome.exe"
    Runs/shuts down the exact same as before.

 B) "out\Release\chrome.exe --force-fieldtrials=BrowserScheduler/Enabled/ --force-fieldtrial-params=BrowserScheduler.Enabled:Background/3;3;1;0;10000/BackgroundFileIO/3;3;1;0;10000/Foreground/3;3;1;0;10000/ForegroundFileIO/3;3;1;0;10000/RedirectSequencedWorkerPools/true/RedirectNonUINonIOBrowserThreads/true"
    Runs/shuts down smoothly and chrome://tracing confirms that redirected BrowserThreads are running on TaskSchedulerWorkers.

Review-Url: https://codereview.chromium.org/2464233002
Cr-Commit-Position: refs/heads/master@{#439139}

[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/content/browser/browser_main_loop.cc
[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/content/browser/browser_main_loop.h
[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/content/browser/browser_thread_impl.cc
[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/content/browser/browser_thread_impl.h
[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/content/public/browser/content_browser_client.h
[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/content/public/test/test_browser_thread.cc
[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/content/public/test/test_browser_thread.h
[modify] https://crrev.com/3ee0e445ae07397a077eb3213fc46318af01b43e/content/public/test/test_browser_thread_bundle.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Dec 17 2016

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

commit 7b5334b8262bbbce140fb2dcc3aeb4034d5d0dbe
Author: gab <gab@chromium.org>
Date: Sat Dec 17 00:05:41 2016

Redirect BrowserThread::FILE at USER_VISIBLE instead of BACKGROUND priority.

An oversight in https://codereview.chromium.org/2464233002/. Although
FILE should mostly be for BACKGROUND work, there are some USER_VISIBLE
tasks on it and as such it has to be redirected as USER_VISIBLE until
migration of individual tasks can clarify per task priority (just like
BlockingPool is redirected as USER_VISIBLE).

BUG= 653916 
TBR=avi@chromium.org
NO_DEPENDENCY_CHECKS=true

Review-Url: https://codereview.chromium.org/2584923002
Cr-Commit-Position: refs/heads/master@{#439240}

[modify] https://crrev.com/7b5334b8262bbbce140fb2dcc3aeb4034d5d0dbe/content/browser/browser_main_loop.cc

Blockedon: 675800
Blockedon: -675800
Project Member

Comment 34 by bugdroid1@chromium.org, Dec 21 2016

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

commit 177138a0430cb79513a26c30f348e6cb9fe3b8e3
Author: gab <gab@chromium.org>
Date: Wed Dec 21 12:56:34 2016

Enable RedirectNonUINonIOBrowserThreads behind --enable-browser-task-scheduler on trunk.

BUG= 653916 
R=rkaplow@chromium.org

Review-Url: https://codereview.chromium.org/2591793003
Cr-Commit-Position: refs/heads/master@{#440077}

[modify] https://crrev.com/177138a0430cb79513a26c30f348e6cb9fe3b8e3/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 35 by bugdroid1@chromium.org, Mar 31 2017

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

commit c6f277128dfba1874ebd3e698ad01254faf9ff9a
Author: gab <gab@chromium.org>
Date: Fri Mar 31 21:06:10 2017

Fix destruction order of FileSystemContext in MediaFileValidatorTest.

Pre-requisite for https://codereview.chromium.org/2690183002/

Otherwise it causes:
[7962:7962:0330/140009.008419:FATAL:task_tracker.cc(394)] Check failed: !shutdown_event_->IsSignaled().
#0 0x000002edc667 base::debug::StackTrace::StackTrace()
#1 0x000002ef500b logging::LogMessage::~LogMessage()
#2 0x000002f3f8b1 base::internal::TaskTracker::BeforePostTask()
#3 0x000002f3f75f base::internal::TaskTracker::WillPostTask()
#4 0x000002f93628 base::internal::SchedulerSingleThreadTaskRunnerManager::SchedulerSingleThreadTaskRunner::PostDelayedTask()
#5 0x000002f4982b base::SequencedWorkerPool::PoolSequencedTaskRunner::PostNonNestableDelayedTask()
#6 0x000002f24e4b base::SequencedTaskRunner::DeleteOrReleaseSoonInternal()
#7 0x000004c6dce5 storage::FileSystemContext::DeleteOnCorrectThread()
#8 0x000000c7232e MediaFileValidatorTest::~MediaFileValidatorTest()
#9 0x000000c722a9 MediaFileValidatorTest_ValidAudio_Test::~MediaFileValidatorTest_ValidAudio_Test()
#10 0x000003d7c391 testing::TestInfo::Run()
#11 0x000003d7c867 testing::TestCase::Run()
#12 0x000003d838a7 testing::internal::UnitTestImpl::RunAllTests()
#13 0x000003d83527 testing::UnitTest::Run()
#14 0x000002fcbc71 base::TestSuite::Run()
#15 0x000002ecef48 ChromeTestSuiteRunner::RunTestSuite()
#16 0x0000035e80ec content::LaunchTests()

BUG= 653916 

Review-Url: https://codereview.chromium.org/2789923002
Cr-Commit-Position: refs/heads/master@{#461227}

[modify] https://crrev.com/c6f277128dfba1874ebd3e698ad01254faf9ff9a/chrome/browser/media_galleries/fileapi/media_file_validator_browsertest.cc

Project Member

Comment 36 by bugdroid1@chromium.org, Apr 3 2017

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

commit b6d0c9a06ab573af760763fcd7ecf76ae2c87695
Author: gab <gab@chromium.org>
Date: Mon Apr 03 18:30:47 2017

RedirectNonUINonIOBrowserThreads to TaskScheduler by default on trunk.

The addition of AssertWaitAllowed in BrowserMainLoop::ShutdownThreadsAndCleanUp()
is required because joining is a "wait" operation. It wasn't required
before because PlatformThread::Join (platform_thread_posix.cc) actually still
does AssertIOAllowed() instead of AssertWaitAllowed() for legacy reasons
(and I/O is allowed in shutdown already). But this CL enables the redirection
which uses a WaitableEvent to mimic thread join (ref. BrowserThreadImpl::StopRedirectionOfThreadID():
waiting on the exact same set of tasks it did prior to redirection).

BUG= 653916 

Review-Url: https://codereview.chromium.org/2690183002
Cr-Commit-Position: refs/heads/master@{#461481}

[modify] https://crrev.com/b6d0c9a06ab573af760763fcd7ecf76ae2c87695/base/threading/thread_restrictions.h
[modify] https://crrev.com/b6d0c9a06ab573af760763fcd7ecf76ae2c87695/content/browser/browser_main_loop.cc
[modify] https://crrev.com/b6d0c9a06ab573af760763fcd7ecf76ae2c87695/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 37 by bugdroid1@chromium.org, Apr 4 2017

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

commit f07732ec0e39ad7e9f1ad279f2624dbdf261803d
Author: robliao <robliao@chromium.org>
Date: Tue Apr 04 00:33:33 2017

Revert of RedirectNonUINonIOBrowserThreads to TaskScheduler by default on trunk. (patchset #3 id:40001 of https://codereview.chromium.org/2690183002/ )

Reason for revert:
Breaks some DBUS code that requests for a message loop

Original issue's description:
> RedirectNonUINonIOBrowserThreads to TaskScheduler by default on trunk.
>
> The addition of AssertWaitAllowed in BrowserMainLoop::ShutdownThreadsAndCleanUp()
> is required because joining is a "wait" operation. It wasn't required
> before because PlatformThread::Join (platform_thread_posix.cc) actually still
> does AssertIOAllowed() instead of AssertWaitAllowed() for legacy reasons
> (and I/O is allowed in shutdown already). But this CL enables the redirection
> which uses a WaitableEvent to mimic thread join (ref. BrowserThreadImpl::StopRedirectionOfThreadID():
> waiting on the exact same set of tasks it did prior to redirection).
>
> BUG= 653916 
>
> Review-Url: https://codereview.chromium.org/2690183002
> Cr-Commit-Position: refs/heads/master@{#461481}
> Committed: https://chromium.googlesource.com/chromium/src/+/b6d0c9a06ab573af760763fcd7ecf76ae2c87695

TBR=jam@chromium.org,rkaplow@chromium.org,gab@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 653916 ,  707986 

Review-Url: https://codereview.chromium.org/2791933004
Cr-Commit-Position: refs/heads/master@{#461588}

[modify] https://crrev.com/f07732ec0e39ad7e9f1ad279f2624dbdf261803d/base/threading/thread_restrictions.h
[modify] https://crrev.com/f07732ec0e39ad7e9f1ad279f2624dbdf261803d/content/browser/browser_main_loop.cc
[modify] https://crrev.com/f07732ec0e39ad7e9f1ad279f2624dbdf261803d/testing/variations/fieldtrial_testing_config.json

Comment 38 by gab@chromium.org, Apr 4 2017

Blockedon: 707986
Blockedon: 707935
Project Member

Comment 41 by bugdroid1@chromium.org, May 1 2017

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

commit d4be08b81e51bbf512152939dce9955133f74864
Author: robliao <robliao@chromium.org>
Date: Mon May 01 23:32:37 2017

Revert of Reland RedirectNonUINonIOBrowserThreads to TaskScheduler by default on trunk (patchset #1 id:1 of https://codereview.chromium.org/2856583002/ )

Reason for revert:
Possible failure: https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.chromiumos%2FLinux_ChromiumOS_Tests__dbg__1_%2F25521%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests%2F0%2Flogs%2FCaptivePortalWindowTest.OnRedirected%2F0

Original issue's description:
> Reland RedirectNonUINonIOBrowserThreads to TaskScheduler by default on trunk
>
> This reverts commit f07732ec0e39ad7e9f1ad279f2624dbdf261803d
> (https://codereview.chromium.org/2791933004)
> and reapplies b6d0c9a06ab573af760763fcd7ecf76ae2c87695
> (https://codereview.chromium.org/2690183002/).
>
> BUG= 653916 ,  707986 
> TBR=jam@chromium.org, rkaplow@chromium.org
> Previously reviewed at https://codereview.chromium.org/2690183002/
>
> Review-Url: https://codereview.chromium.org/2856583002
> Cr-Commit-Position: refs/heads/master@{#468376}
> Committed: https://chromium.googlesource.com/chromium/src/+/3e27cb6e501ae55d614b56d566487590518ec223

TBR=jam@chromium.org,rkaplow@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 653916 ,  707986 

Review-Url: https://codereview.chromium.org/2848413002
Cr-Commit-Position: refs/heads/master@{#468481}

[modify] https://crrev.com/d4be08b81e51bbf512152939dce9955133f74864/base/threading/thread_restrictions.h
[modify] https://crrev.com/d4be08b81e51bbf512152939dce9955133f74864/content/browser/browser_main_loop.cc
[modify] https://crrev.com/d4be08b81e51bbf512152939dce9955133f74864/testing/variations/fieldtrial_testing_config.json

Blockedon: 717380
Blockedon: 698140
Blockedon: -698140
Project Member

Comment 46 by bugdroid1@chromium.org, May 8 2017

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

commit 0e108ddb699df8f9b1cbd409cf2677739b4058a1
Author: robliao <robliao@chromium.org>
Date: Mon May 08 20:05:44 2017

Add Redirection For BrowserThread::FILE to the COM STA Task Runner

Now that the COM STA Task Runner is available, BrowserThread::FILE can
use it on Windows.

BUG= 653916 

Review-Url: https://codereview.chromium.org/2791503002
Cr-Commit-Position: refs/heads/master@{#470096}

[modify] https://crrev.com/0e108ddb699df8f9b1cbd409cf2677739b4058a1/content/browser/browser_main_loop.cc

The commit from comment #45 causes debug builds to reliably crash here on Linux with KDE and its proxy settings set to "detect proxy configuration automatically":

[36266:36287:0509/115733.764591:FATAL:message_loop.h(539)] Check failed: loop. 
#0  0x00007ffff7558547 in base::debug::(anonymous namespace)::DebugBreak() () at ../../base/debug/debugger_posix.cc:232
#1  0x00007ffff7558528 in base::debug::BreakDebugger() () at ../../base/debug/debugger_posix.cc:251
#2  0x00007ffff75ce1f4 in logging::LogMessage::~LogMessage() (this=0x7fffc0cc1118) at ../../base/logging.cc:783
#3  0x00007ffff6652939 in base::MessageLoopForIO::current() () at ../../base/message_loop/message_loop.h:539
#4  0x00007ffff6aebe30 in net::(anonymous namespace)::SettingGetterImplKDE::SetUpNotifications(net::ProxyConfigServiceLinux::Delegate*) (this=0x2b08598e59c0, delegate=0x2b085987dfa0) at ../../net/proxy/proxy_config_service_linux.cc:995
#5  0x00007ffff6ae529b in net::ProxyConfigServiceLinux::Delegate::SetUpNotifications() (this=0x2b085987dfa0) at ../../net/proxy/proxy_config_service_linux.cc:1666
#6  0x00007ffff6af51d7 in base::internal::FunctorTraits<void (net::ProxyConfigServiceLinux::Delegate::*)(), void>::Invoke<scoped_refptr<net::ProxyConfigServiceLinux::Delegate> const&>(void (net::ProxyConfigServiceLinux::Delegate::*)(), scoped_refptr<net::ProxyConfigServiceLinux::Delegate> const&) (method=(void (net::ProxyConfigServiceLinux::Delegate::*)(net::ProxyConfigServiceLinux::Delegate * const)) 0x7ffff6ae5150 <net::ProxyConfigServiceLinux::Delegate::SetUpNotifications()>, receiver_ptr=scoped_refptr((net::ProxyConfigServiceLinux::Delegate *)0x2b085987dfa0)) at ../../base/bind_internal.h:214
#7  0x00007ffff6af5121 in base::internal::InvokeHelper<false, void>::MakeItSo<void (net::ProxyConfigServiceLinux::Delegate::* const&)(), scoped_refptr<net::ProxyConfigServiceLinux::Delegate> const&>(void (net::ProxyConfigServiceLinux::Delegate::* const&)(), scoped_refptr<net::ProxyConfigServiceLinux::Delegate> const&) (functor=@0x2b08593706c8: (void (net::ProxyConfigServiceLinux::Delegate::*)(net::ProxyConfigServiceLinux::Delegate * const)) 0x7ffff6ae5150 <net::ProxyConfigServiceLinux::Delegate::SetUpNotifications()>, args=scoped_refptr((net::ProxyConfigServiceLinux::Delegate *)0x2b085987dfa0)) at ../../base/bind_internal.h:285
#8  0x00007ffff6af50c2 in base::internal::Invoker<base::internal::BindState<void (net::ProxyConfigServiceLinux::Delegate::*)(), scoped_refptr<net::ProxyConfigServiceLinux::Delegate> >, void ()>::RunImpl<void (net::ProxyConfigServiceLinux::Delegate::* const&)(), std::tuple<scoped_refptr<net::ProxyConfigServiceLinux::Delegate> > const&, 0ul>(void (net::ProxyConfigServiceLinux::Delegate::* const&)(), std::tuple<scoped_refptr<net::ProxyConfigServiceLinux::Delegate> > const&, base::IndexSequence<0ul>) (functor=@0x2b08593706c8: (void (net::ProxyConfigServiceLinux::Delegate::*)(net::ProxyConfigServiceLinux::Delegate * const)) 0x7ffff6ae5150 <net::ProxyConfigServiceLinux::Delegate::SetUpNotifications()>, bound=std::tuple containing = {...}) at ../../base/bind_internal.h:361
#9  0x00007ffff6af500c in base::internal::Invoker<base::internal::BindState<void (net::ProxyConfigServiceLinux::Delegate::*)(), scoped_refptr<net::ProxyConfigServiceLinux::Delegate> >, void ()>::Run(base::internal::BindStateBase*) (base=0x2b08593706a0) at ../../base/bind_internal.h:339
#10 0x00007ffff751702e in base::Callback<void (), (base::internal::CopyMode)0, (base::internal::RepeatMode)0>::Run() && (this=0x2b08591cd788) at ../../base/callback.h:91
#11 0x00007ffff756149e in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) (this=0x7fffc0cc1d20, queue_function=0x7ffff78ab680 <base::internal::(anonymous namespace)::kQueueFunctionName> "base::PostTask", pending_task=0x2b08591cd770) at ../../base/debug/task_annotator.cc:59
#12 0x00007ffff77214e8 in base::internal::TaskTracker::PerformRunTask(std::unique_ptr<base::internal::Task, std::default_delete<base::internal::Task> >) (this=0x2b0859139518, task=std::unique_ptr<base::internal::Task> containing 0x2b08591cd770) at ../../base/task_scheduler/task_tracker.cc:331
#13 0x00007ffff7723705 in base::internal::TaskTrackerPosix::PerformRunTask(std::unique_ptr<base::internal::Task, std::default_delete<base::internal::Task> >) (this=0x2b0859139518, task=std::unique_ptr<base::internal::Task> containing 0x0) at ../../base/task_scheduler/task_tracker_posix.cc:21
#14 0x00007ffff772097a in base::internal::TaskTracker::RunTask(std::unique_ptr<base::internal::Task, std::default_delete<base::internal::Task> >, base::SequenceToken const&) (this=0x2b0859139518, task=std::unique_ptr<base::internal::Task> containing 0x0, sequence_token=...) at ../../base/task_scheduler/task_tracker.cc:293
#15 0x00007ffff770dfcb in base::internal::SchedulerWorker::Thread::ThreadMain() (this=0x2b0859a843e0) at ../../base/task_scheduler/scheduler_worker.cc:79
#16 0x00007ffff772f33a in base::(anonymous namespace)::ThreadFunc(void*) (params=0x2b085958bed0) at ../../base/threading/platform_thread_posix.cc:71
#17 0x00007ffff7bc06ca in start_thread (arg=0x7fffc0cc3700) at pthread_create.c:333
#18 0x00007fffe00ccf7f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:105
Oh, I've just checked and release builds are equally broken here.

Comment 49 by gab@chromium.org, May 9 2017

Cc: -robliao@chromium.org gab@chromium.org
Owner: robliao@chromium.org
Status: Started (was: Assigned)
robliao@ is completing redirection of BrowserThreads

Comment 50 by gab@chromium.org, May 9 2017

Cc: robliao@chromium.org
Owner: gab@chromium.org
Never mind, robliao@ is busy today, I'll jump on this, fix incoming for #47.
Project Member

Comment 51 by bugdroid1@chromium.org, May 10 2017

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

commit f4f904edc4cbbb9ffd269cd6fd5d26d9925aee8c
Author: gab <gab@chromium.org>
Date: Wed May 10 20:55:02 2017

Use base::FileDescriptorWatcher instead of MessageLoopForIO::WatchFileDescriptor in proxy_config_service_linux.cc

base::FileDescriptorWatcher is the equivalent MessageLoop independent API
(the only difference is an extra PostTask hop on notification which is irrelevant
for low volume handles)

This MessageLoopForIO::WatchFileDescriptor is causing problems @ https://bugs.chromium.org/p/chromium/issues/detail?id=653916#c47
per the FILE thread's MessageLoop going away.

BUG= 645114 ,  653916 

Review-Url: https://codereview.chromium.org/2872863003
Cr-Commit-Position: refs/heads/master@{#470702}

[modify] https://crrev.com/f4f904edc4cbbb9ffd269cd6fd5d26d9925aee8c/net/proxy/proxy_config_service_linux.cc

Comment 52 by gab@chromium.org, May 15 2017

Status: Fixed (was: Started)
This appears to have stuck \o/!!

Comment 53 by gab@chromium.org, May 15 2017

Blocking: 722536
Project Member

Comment 54 by bugdroid1@chromium.org, Jun 14 2017

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

commit 664e448e46c5f7fe42b25e5c27f97973861e2b09
Author: Gabriel Charette <gab@chromium.org>
Date: Wed Jun 14 16:39:25 2017

Add presubmit against usage of deprecated BrowserThreads.

Following announcement @ https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/_X8yyHP3uwI

Bug:  653916 
Change-Id: I1c6a4dc03adaf45ed8f28554e5d85139e782502a
Reviewed-on: https://chromium-review.googlesource.com/533806
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479416}
[modify] https://crrev.com/664e448e46c5f7fe42b25e5c27f97973861e2b09/PRESUBMIT.py

Project Member

Comment 55 by bugdroid1@chromium.org, Jun 14 2017

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

commit c0cdb67650c9f043762a84493362950e1dc8041b
Author: Gabriel Charette <gab@chromium.org>
Date: Wed Jun 14 16:41:04 2017

Cleanup non-UI/IO BrowserThreads redirection experiment.

Already was default through configs, removing experiment and making it so
for realz on trunk.

Also removed ShouldRedirectDOMStorageTaskRunner() left behind by a prior cleanup.

Bug:  653916 
Change-Id: I08fb194d6ddeae52ce444ac977654df30f71b75f
Reviewed-on: https://chromium-review.googlesource.com/533654
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479418}
[modify] https://crrev.com/c0cdb67650c9f043762a84493362950e1dc8041b/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/c0cdb67650c9f043762a84493362950e1dc8041b/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/c0cdb67650c9f043762a84493362950e1dc8041b/content/browser/browser_main_loop.cc
[modify] https://crrev.com/c0cdb67650c9f043762a84493362950e1dc8041b/content/browser/browser_main_loop.h
[modify] https://crrev.com/c0cdb67650c9f043762a84493362950e1dc8041b/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/c0cdb67650c9f043762a84493362950e1dc8041b/content/public/browser/content_browser_client.h
[modify] https://crrev.com/c0cdb67650c9f043762a84493362950e1dc8041b/testing/variations/fieldtrial_testing_config.json

Sign in to add a comment