Add AssertNoURLRequests() to URLRequestContext subclass destructors.

Add AssertNoURLRequests() to URLRequestContext subclass destructors.

BUG=377852
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284556

[Johnny, 2014-07-10 16:41:46]

+rsleevi for chrome/browser/net/ and net/
+sky for chrome/browser/, chrome/test/, and content/public/test/
+tim for sync/

Thanks.

[Ryan Sleevi, 2014-07-10 16:49:20]

net/ and c/b/net LGTM.

Seems overkill to update the tests too, but I'll buy it for consistency.

https://codereview.chromium.org/379293002/diff/20001/chrome/browser/io_thread.cc
File chrome/browser/io_thread.cc (right):

https://codereview.chromium.org/379293002/diff/20001/chrome/browser/io_thread...
chrome/browser/io_thread.cc:162: AssertNoURLRequests();
Seems like this should go second, right?

After all, what if line 164 cancels requests?

[Johnny, 2014-07-10 16:58:29]

https://codereview.chromium.org/379293002/diff/20001/chrome/browser/io_thread.cc
File chrome/browser/io_thread.cc (right):

https://codereview.chromium.org/379293002/diff/20001/chrome/browser/io_thread...
chrome/browser/io_thread.cc:162: AssertNoURLRequests();
On 2014/07/10 16:49:19, Ryan Sleevi wrote:
> Seems like this should go second, right?
> 
> After all, what if line 164 cancels requests?

That'd be a violation of the URLRequestContext lifetime invariant. URLRequests
must keep a URLRequestContext alive, not the other way around, no?

(It doesn't, btw. Just sets a global).

[sky, 2014-07-10 19:37:33]

https://codereview.chromium.org/379293002/diff/20001/net/url_request/url_requ...
File net/url_request/url_request_context.h (right):

https://codereview.chromium.org/379293002/diff/20001/net/url_request/url_requ...
net/url_request/url_request_context.h:204: // prior to implicit destruction of
subclass-owned state.
How come the call to this in the destructor isn't enough?

[Ryan Sleevi, 2014-07-10 19:39:36]

https://codereview.chromium.org/379293002/diff/20001/net/url_request/url_requ...
File net/url_request/url_request_context.h (right):

https://codereview.chromium.org/379293002/diff/20001/net/url_request/url_requ...
net/url_request/url_request_context.h:204: // prior to implicit destruction of
subclass-owned state.
On 2014/07/10 19:37:32, sky wrote:
> How come the call to this in the destructor isn't enough?

The issue is running into crashes in the derived-classes due to URLRequests
still living, for which their DTORs run before the base class (which calls this)

By moving it into the derived classes (as well), this avoids even more
mysterious crash debugging and making the culprit explicit.

[Johnny, 2014-07-10 20:00:05]

Added a couple of subclasses of subclasses that I missed.

-tim (OOO), +nick for sync/
+mmenke for chrome/browser/profiles/profile_io_data.cc

Thanks

[mmenke, 2014-07-10 20:03:00]

On 2014/07/10 20:00:05, Johnny wrote:
> Added a couple of subclasses of subclasses that I missed.
> 
> -tim (OOO), +nick for sync/
> +mmenke for chrome/browser/profiles/profile_io_data.cc
> 
> Thanks

profiles/ LGTM!  Thanks for doing this.

[sky, 2014-07-10 20:05:52]

Ok, LGTM

[Johnny, 2014-07-21 19:51:55]

Nick: Ping? Thanks

[Nicolas Zea, 2014-07-21 20:09:07]

Nick is on leave I believe. Sync LGTM

[Johnny, 2014-07-21 20:14:00]

The CQ bit was checked by jgraettinger@chromium.org

[commit-bot: I haz the power, 2014-07-21 20:15:53]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/jgraettinger@chromium.org/379293002/40001

[commit-bot: I haz the power, 2014-07-22 00:27:53]

Change committed as 284556