New issue
Advanced search Search tips

Issue 859095 link

Starred by 1 user
Status: Fixed
Owner:
w...@chromium.org
Closed: Sep 22
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac , Fuchsia
Pri: 3
Type: Bug



Sign in to add a comment

Remove RunLoop::QuitCurrent*Deprecated() calls in production code, and update [D]CHECKs

Project Member Reported by w...@chromium.org, Jun 29 2018 
 Issue 844016  tracked some cleanups of browser process main message loop termination. Some remaining cleanups are tracked here, in approximate order of priority.

1. Remove remaining calls to QuitCurrent*Deprecated() in production code, and reduce the CHECKs to DCHECKs.
2. Fix tests which mix QuitClosure with QuitCurrentWhenIdleClosureDeprecated(), and add a DCHECK to that call as well.
3. Gradually migrate test call-sites off of QuitCurrent*Deprecated().

Comment 1 by w...@chromium.org, Aug 3

Status: Assigned (was: Started)

Comment 3 by w...@chromium.org, Sep 1

Status: Started (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 4 

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

commit 531a78f0364f8b12134a640f2a26c8f8ec5d3167
Author: Wez <wez@chromium.org>
Date: Tue Sep 04 16:33:32 2018

Migrate remoting Me2Me host off of QuitCurrent*Deprecated().

Bug:  859095 
Change-Id: Ieeb5af5b73339e468256365f574db98374ee9dbf
Reviewed-on: https://chromium-review.googlesource.com/1200836
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588539}
[modify] https://crrev.com/531a78f0364f8b12134a640f2a26c8f8ec5d3167/remoting/host/remoting_me2me_host.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 4 

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

commit 531a78f0364f8b12134a640f2a26c8f8ec5d3167
Author: Wez <wez@chromium.org>
Date: Tue Sep 04 16:33:32 2018

Migrate remoting Me2Me host off of QuitCurrent*Deprecated().

Bug:  859095 
Change-Id: Ieeb5af5b73339e468256365f574db98374ee9dbf
Reviewed-on: https://chromium-review.googlesource.com/1200836
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588539}
[modify] https://crrev.com/531a78f0364f8b12134a640f2a26c8f8ec5d3167/remoting/host/remoting_me2me_host.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 5 

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

commit f833443c1363d24ec63192f6df7f1024daf9ada9
Author: Wez <wez@chromium.org>
Date: Tue Sep 04 17:30:36 2018

Migrate synchronous host resolver off QuitCurrent*Deprecated().

Bug:  859095 
Change-Id: Ic3aad8b15330705f939677c7d7447cd022ab00ca
Reviewed-on: https://chromium-review.googlesource.com/1200837
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588569}
[modify] https://crrev.com/f833443c1363d24ec63192f6df7f1024daf9ada9/net/tools/quic/synchronous_host_resolver.cc
[modify] https://crrev.com/f833443c1363d24ec63192f6df7f1024daf9ada9/net/tools/quic/synchronous_host_resolver.h
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 7 

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

commit dd1a0321f55f57f94063caab9535633e87a01325
Author: Wez <wez@chromium.org>
Date: Fri Sep 07 01:17:32 2018

Migrate MockAdbServer off of QuitCurrent*Deprecated().

Bug:  859095 
Change-Id: Ibe8a52dfacad07b1cb9f04e720af063f67f9a3b0
Reviewed-on: https://chromium-review.googlesource.com/1200838
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589398}
[modify] https://crrev.com/dd1a0321f55f57f94063caab9535633e87a01325/chrome/browser/devtools/device/adb/mock_adb_server.cc
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 7 

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

commit 6979109b593d10aef2e923e777c421bc7f749a93
Author: Wez <wez@chromium.org>
Date: Fri Sep 07 17:30:56 2018

Migrate non-test code in //content off of QuitCurrent*Deprecated().

- ChildThreadImpl now takes a |quit_closure|, which is provided by all
  sub-classes when used out-of-process.
- Sub-classes which can run in-process instead pass base::DoNothing() to
  ChildThreadImpl, to instead rely on the in-browser Thread having
  Stop() invoked on it, to shut them down.

Bug:  859095 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I0251db64063601a6a9de8883c8558e25104e5b0b
Reviewed-on: https://chromium-review.googlesource.com/1200839
Reviewed-by: enne <enne@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589564}
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/components/viz/service/main/viz_main_impl.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/components/viz/service/main/viz_main_impl.h
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/child/child_thread_impl.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/child/child_thread_impl.h
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/gpu/gpu_child_thread.h
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/gpu/gpu_main.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/ppapi_plugin/ppapi_broker_main.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/ppapi_plugin/ppapi_plugin_main.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/ppapi_plugin/ppapi_thread.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/ppapi_plugin/ppapi_thread.h
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/renderer/pepper/ppb_flash_message_loop_impl.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/renderer/render_thread_impl.h
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/renderer/renderer_main.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/utility/utility_main.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/utility/utility_thread_impl.cc
[modify] https://crrev.com/6979109b593d10aef2e923e777c421bc7f749a93/content/utility/utility_thread_impl.h

Comment 9 by w...@chromium.org, Sep 13 

QuitCurrentWhenIdleClosureDeprecated() and QuitCurrentDeprecated() are no longer in-use other than tests.
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 21 

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

commit 53f3d30ef104e4c91ab0d7eb9683305139072ef3
Author: Wez <wez@chromium.org>
Date: Fri Sep 21 16:55:16 2018

Migrate example & mock call-sites off QuitCurrent*Deprecated().

- Migrate //mash and //ui/views examples off QuitCurrent*Deprecated().
- Migrate //net mock off it as well.

These are the final remaining call-sites that codesearch identifies as
not-test-specific (even though the mock is a mock ;).

Bug:  859095 
Change-Id: I417f9c93aa6ef6eaae4cf22948e07750d176d83d
Reviewed-on: https://chromium-review.googlesource.com/1226455
Commit-Queue: Wez <wez@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593230}
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/ash/shell/app_list.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/ash/shell/content/client/shell_browser_main_parts.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/mash/catalog_viewer/catalog_viewer.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/mash/example/views_examples/views_examples.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/mash/example/window_type_launcher/window_type_launcher.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/mash/task_viewer/task_viewer.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/net/proxy_resolution/mock_pac_file_fetcher.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/net/proxy_resolution/mock_pac_file_fetcher.h
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/ui/views/examples/examples_main.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/ui/views/examples/examples_window.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/ui/views/examples/examples_window.h
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/ui/views/examples/examples_window_with_content.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/ui/views/examples/examples_window_with_content.h
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/ui/views/examples/examples_with_content_main_exe.cc
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/ui/views_content_client/views_content_client.h
[modify] https://crrev.com/53f3d30ef104e4c91ab0d7eb9683305139072ef3/ui/views_content_client/views_content_client_main_parts.cc

Comment 11 by w...@chromium.org, Sep 22

Status: Fixed (was: Started)
Closing this as Fixed now that the production code call-sites have been purged.

We haven't reinstated the DCHECK in QuitCurrentWhenIdleClosureDeprecated(), since that triggers false-alarms in too many tests; we can address that incrementally as tests are updated in future, if we want.
►

Sign in to add a comment