I wonder if we should change ~VizMainImpl to add a RunLoop on the GPU thread while the VizCompositorThread shuts down? I tried this originally and failed, although I think it was due to not closing all the right Mojo message pipes which post non-nestable tasks.

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

commit 23af711fb1f114b12ac853842ceb0c307e77eb82
Author: Eric Karl <ericrk@chromium.org>
Date: Fri Nov 02 20:47:34 2018

OOP-D: Fix deadlock during Viz thread shutdown

Currently, VizMainImpl::ExitProcess has a workaround to avoid deadlock
when shutting down the gpu process. Unfortunately, this is only wired up
to one of the ways to shut down. Other shutdown calls originating from
ChildProcessImpl miss this logic and hit the same deadlock.

This change passes a helper to ChildProcessImpl, allowing it to correctly
handle shutdown. This helper posts a task to the GPU main thread before
starting shutdown to ensure both that we're on the right thread as well
as that we're post-init.

Bug:  901396 
Change-Id: I58cff914def5c05f7b9e6b6f58cb319153d685dd
Reviewed-on: https://chromium-review.googlesource.com/c/1315538
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605043}
[modify] https://crrev.com/23af711fb1f114b12ac853842ceb0c307e77eb82/components/viz/service/main/viz_main_impl.h
[modify] https://crrev.com/23af711fb1f114b12ac853842ceb0c307e77eb82/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/23af711fb1f114b12ac853842ceb0c307e77eb82/content/gpu/gpu_child_thread.h

The crash signature hasn't been seen since landing the fix.

As the fix has baked for a few days now, and this his is a pretty clear deadlock issue with a fairly scoped fix, I'd like to merge to M71.

This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Pls apply appropriate OSs label.

Sorry, this is an Android issue.

Re. safety - this is a very scoped change in cases where VizDisplayCompositor is not turned on. As we're rolling VizDisplayCompositor out now, we can always disable it if a significant issue is introduced. In non-VizDisplayController cases, this change introduces a thread hop, but no functionality difference.

Re. impact - this fix is lowish impact - around 0.06 - 0.2 CPM, so might not be worth it from that perspective. On the other hand, this ins't flaky, but a well-understood deadlock that is hittable.

+benmason@ (Chrome on Android TPM) for M71 merge review, ptal comment #7. Thank you.

Approved for merge to 71, branch 3578.

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

Commit: 977a65455b60c80166ebca5bcbd852829fb86d43
Author: ericrk@chromium.org
Commiter: ericrk@chromium.org
Date: 2018-11-13 23:17:55 +0000 UTC

OOP-D: Fix deadlock during Viz thread shutdown

Currently, VizMainImpl::ExitProcess has a workaround to avoid deadlock
when shutting down the gpu process. Unfortunately, this is only wired up
to one of the ways to shut down. Other shutdown calls originating from
ChildProcessImpl miss this logic and hit the same deadlock.

This change passes a helper to ChildProcessImpl, allowing it to correctly
handle shutdown. This helper posts a task to the GPU main thread before
starting shutdown to ensure both that we're on the right thread as well
as that we're post-init.

TBR=ericrk@chromium.org

(cherry picked from commit 23af711fb1f114b12ac853842ceb0c307e77eb82)

Bug:  901396 
Change-Id: I58cff914def5c05f7b9e6b6f58cb319153d685dd
Reviewed-on: https://chromium-review.googlesource.com/c/1315538
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#605043}
Reviewed-on: https://chromium-review.googlesource.com/c/1334655
Reviewed-by: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#672}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

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

commit 977a65455b60c80166ebca5bcbd852829fb86d43
Author: Eric Karl <ericrk@chromium.org>
Date: Tue Nov 13 23:17:55 2018

OOP-D: Fix deadlock during Viz thread shutdown

Currently, VizMainImpl::ExitProcess has a workaround to avoid deadlock
when shutting down the gpu process. Unfortunately, this is only wired up
to one of the ways to shut down. Other shutdown calls originating from
ChildProcessImpl miss this logic and hit the same deadlock.

This change passes a helper to ChildProcessImpl, allowing it to correctly
handle shutdown. This helper posts a task to the GPU main thread before
starting shutdown to ensure both that we're on the right thread as well
as that we're post-init.

TBR=ericrk@chromium.org

(cherry picked from commit 23af711fb1f114b12ac853842ceb0c307e77eb82)

Bug:  901396 
Change-Id: I58cff914def5c05f7b9e6b6f58cb319153d685dd
Reviewed-on: https://chromium-review.googlesource.com/c/1315538
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#605043}
Reviewed-on: https://chromium-review.googlesource.com/c/1334655
Reviewed-by: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#672}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/977a65455b60c80166ebca5bcbd852829fb86d43/components/viz/service/main/viz_main_impl.h
[modify] https://crrev.com/977a65455b60c80166ebca5bcbd852829fb86d43/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/977a65455b60c80166ebca5bcbd852829fb86d43/content/gpu/gpu_child_thread.h