[Cronet] Leak CronetHttpProtocolHandlerDelegate on shutdown.

[Cronet] Leak CronetHttpProtocolHandlerDelegate on shutdown.
Prevent crash if background threads are handling requests while main is exiting.

BUG=706580
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2778743010
Cr-Commit-Position: refs/heads/master@{#460904}
Committed: https://chromium.googlesource.com/chromium/src/+/65d11a7c591307ff2d10145a07655b53010e04d0

[mef, 2017-03-29 22:41:21]

Description was changed from

==========
[Cronet] Leak CronetHttpProtocolHandlerDelegate on shutdown.
Prevent crash if background threads are handling requests while main is exiting.

BUG=706580
==========

to

==========
[Cronet] Leak CronetHttpProtocolHandlerDelegate on shutdown.
Prevent crash if background threads are handling requests while main is exiting.

BUG=706580
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

[mef, 2017-03-29 22:44:36]

mef@chromium.org changed reviewers:
+ kapishnikov@chromium.org, lilyhoughton@chromium.org

[mef, 2017-03-29 22:44:36]

PTAL, this is a fix for Cronet crash on app shutdown on iOS.

I was able to reproduce it by reseting gHttpProtocolHandlerDelegate unique
pointer.

[kapishnikov, 2017-03-30 17:23:02]

It looks that the solution is going to work for NSURLConnection-s and shared
NSURLSession-s only. There is still need to address the case when the client
instantiates NSURLSession with a custom NSURLSessionConfiguration.

What if instead adding logic to unregisterHttpProtocolHandler(), we add a new
[Cronet shutdown] method that will mark the engine as shut down and make sure
that we don't call RequestFilterBlock is that state.

[mef, 2017-03-30 20:18:09]

On 2017/03/30 17:23:02, kapishnikov wrote:
> It looks that the solution is going to work for NSURLConnection-s and shared
> NSURLSession-s only. There is still need to address the case when the client
> instantiates NSURLSession with a custom NSURLSessionConfiguration.
> 
> What if instead adding logic to unregisterHttpProtocolHandler(), we add a new
> [Cronet shutdown] method that will mark the engine as shut down and make sure
> that we don't call RequestFilterBlock is that state.

Per offline discussion I've removed changes to unregisterHttpProtocolHandler()
and tested leaky fix separately using sample app in
https://codereview.chromium.org/2771633002/

PTAL.

[kapishnikov, 2017-03-30 20:20:18]

lgtm

[mef, 2017-03-30 21:01:07]

The CQ bit was checked by mef@chromium.org

[commit-bot: I haz the power, 2017-03-30 21:03:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 22:13:36]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490907667155420,
"parent_rev": "dc19a899c0f97dcd259a3ec506396c43db14ef23", "commit_rev":
"65d11a7c591307ff2d10145a07655b53010e04d0"}

[commit-bot: I haz the power, 2017-03-30 22:15:10]

Description was changed from

==========
[Cronet] Leak CronetHttpProtocolHandlerDelegate on shutdown.
Prevent crash if background threads are handling requests while main is exiting.

BUG=706580
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
==========

to

==========
[Cronet] Leak CronetHttpProtocolHandlerDelegate on shutdown.
Prevent crash if background threads are handling requests while main is exiting.

BUG=706580
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester

Review-Url: https://codereview.chromium.org/2778743010
Cr-Commit-Position: refs/heads/master@{#460904}
Committed:
https://chromium.googlesource.com/chromium/src/+/65d11a7c591307ff2d10145a0765...
==========

[commit-bot: I haz the power, 2017-03-30 22:15:11]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/65d11a7c591307ff2d10145a0765...