Change the update_client task runner behavior to continue on shutdown.

Change the update_client task runner behavior to continue on shutdown.

Historically, we've had issues with running the BITS COM client on
threads and interfering with the browser shutdown.

For reason not entirely understood, some out of process COM calls,
and most common the call to enumerate BITS jobs appear to hang and
consequently trigger the browser hang shutdown detector.

At first, we had run this code on the FILE thread, then we had moved
it on blocking pool threads. However, the net effect is that the
hang moved as well.

This change avoids blocking the shutdown by allowing the code to
run after the browser shutdown until the OS terminates the thread as
part of the process exit.

While it is somehow difficult to reason about the correctness of
the update_client code, this change is reasonably safe to make
due to aspects of refcounting and containment of the update_client
such as not accessing browser global state and refcounting the
objects that are thread aware.

BUG=552028
Committed: https://crrev.com/5081941b99d37952e1c60c9cb24257369d9c6416
Cr-Commit-Position: refs/heads/master@{#359138}

[Sorin Jianu, 2015-11-10 01:03:01]

Description was changed from

==========
Change the update_client task runner behavior to continue on shutdown.

Historically, we've had issues with running the BITS COM client on
threads and interfering with the browser shutdown.

For reason not entirely understood, some out of process COM calls,
and most common the call to enumerate BITS jobs appear to hang and
consequently trigger the browser hang shutdown detector.

At first, we had run this code on the FILE thread, then we had moved
it on blocking pool threads. However, the net effect is that the
hang moved as well.

This change avoids blocking the shutdown by allowing the code to
run after the browser shutdown until the OS terminates the thread as
part of the process exit.

While it is somehow difficult to reason about the correctness of
the update_client code, this change is reasonably safe to make
due to aspects of refcounting and containment of the update_client
such as not accessing browser global state and refcounting the
objects that are thread aware.

BUG=552028
==========

to

==========
Change the update_client task runner behavior to continue on shutdown.

Historically, we've had issues with running the BITS COM client on
threads and interfering with the browser shutdown.

For reason not entirely understood, some out of process COM calls,
and most common the call to enumerate BITS jobs appear to hang and
consequently trigger the browser hang shutdown detector.

At first, we had run this code on the FILE thread, then we had moved
it on blocking pool threads. However, the net effect is that the
hang moved as well.

This change avoids blocking the shutdown by allowing the code to
run after the browser shutdown until the OS terminates the thread as
part of the process exit.

While it is somehow difficult to reason about the correctness of
the update_client code, this change is reasonably safe to make
due to aspects of refcounting and containment of the update_client
such as not accessing browser global state and refcounting the
objects that are thread aware.

BUG=552028
==========

[Sorin Jianu, 2015-11-10 01:03:01]

sorin@chromium.org changed reviewers:
+ waffles@chromium.org

[waffles, 2015-11-10 01:18:28]

lgtm

[Sorin Jianu, 2015-11-10 01:27:00]

The CQ bit was checked by sorin@chromium.org

[commit-bot: I haz the power, 2015-11-10 01:30:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1415933011/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1415933011/20001

[commit-bot: I haz the power, 2015-11-10 02:47:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-10 02:47:01]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Sorin Jianu, 2015-11-10 05:28:49]

sorin@chromium.org changed reviewers:
+ asargent@chromium.org

[Sorin Jianu, 2015-11-10 05:30:05]

Antony, please, I need an owner's review for
extensions/browser/updater/update_service_unittest.cc.

[asargent_no_longer_on_chrome, 2015-11-10 19:53:54]

update_service_unittest.cc lgm

[Sorin Jianu, 2015-11-10 20:45:18]

The CQ bit was checked by sorin@chromium.org

[commit-bot: I haz the power, 2015-11-10 20:47:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1415933011/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1415933011/20001

[commit-bot: I haz the power, 2015-11-10 20:59:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-10 20:59:37]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Sorin Jianu, 2015-11-10 22:10:01]

Antony, PTAL, there was a typo in the owner's approval and the approval was
invalid.

[Sorin Jianu, 2015-11-11 18:02:58]

sorin@chromium.org changed reviewers:
+ rockot@chromium.org

[Sorin Jianu, 2015-11-11 18:04:41]

Ken, please, I need an owner's approval for
extensions/browser/updater/update_service_unittest.cc

Antony's approval was ineffective due to a typo, and I must land this asap,
thank you so much, and sorry for the nag.

[Ken Rockot(use gerrit already), 2015-11-11 18:05:52]

On 2015/11/11 at 18:04:41, sorin wrote:
> Ken, please, I need an owner's approval for
extensions/browser/updater/update_service_unittest.cc
> 
> Antony's approval was ineffective due to a typo, and I must land this asap,
thank you so much, and sorry for the nag.

Oops, rs lgtm

[Sorin Jianu, 2015-11-11 18:08:26]

The CQ bit was checked by sorin@chromium.org

[commit-bot: I haz the power, 2015-11-11 18:09:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1415933011/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1415933011/20001

[commit-bot: I haz the power, 2015-11-11 18:18:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-11 18:18:07]

Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Sorin Jianu, 2015-11-11 18:52:45]

The CQ bit was checked by sorin@chromium.org

[commit-bot: I haz the power, 2015-11-11 18:53:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1415933011/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1415933011/20001

[commit-bot: I haz the power, 2015-11-11 19:35:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-12 19:59:37]

Patchset 2 (id:??) landed as
https://crrev.com/5081941b99d37952e1c60c9cb24257369d9c6416
Cr-Commit-Position: refs/heads/master@{#359138}

[Sorin Jianu, 2015-11-12 23:52:38]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1439153002/ by sorin@chromium.org.

The reason for reverting is: This change is a suspect for causing
crbug.com/475872.