Notify SystemURLRequestContextGetter before shutdown on iOS.

Notify SystemURLRequestContextGetter before shutdown on iOS.

BUG=418060
Committed: https://crrev.com/4bf02abe2596e899205a914e63cad289992cf7f7
Cr-Commit-Position: refs/heads/master@{#408215}

[mef, 2016-07-25 22:19:03]

The CQ bit was checked by mef@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 22:19:28]

Dry run: 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, 2016-07-25 22:21:13]

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

[commit-bot: I haz the power, 2016-07-25 22:21:13]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mef, 2016-07-25 22:22:22]

mef@chromium.org changed reviewers:
+ justincohen@chromium.org, mmenke@chromium.org

[mef, 2016-07-25 22:22:23]

PTAL, I've verified that test triggers AssertNoURLRequests without the fix and
doesn't trigger it with the fix.

[justincohen, 2016-07-25 22:23:45]

lgtm, does it matter that there's no corresponding gyp change?

[mef, 2016-07-25 22:29:45]

The CQ bit was checked by mef@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 22:30:13]

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

[mef, 2016-07-25 22:31:21]

On 2016/07/25 22:23:45, justincohen wrote:
> lgtm, does it matter that there's no corresponding gyp change?

Thanks! That's a good point, I forgot that gyp is still a thing. :)

[mef, 2016-07-27 14:24:36]

The CQ bit was checked by mef@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 14:24:55]

Dry run: 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, 2016-07-27 15:25:35]

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

[commit-bot: I haz the power, 2016-07-27 15:25:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mmenke, 2016-07-27 17:23:46]

LGTM

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
File ios/chrome/browser/ios_chrome_io_thread.mm (right):

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
ios/chrome/browser/ios_chrome_io_thread.mm:234: if (shutting_down_)
Could null out io_thread_ instead, don't think it really matters, either way.

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
File ios/chrome/browser/ios_chrome_io_thread_unittest.mm (right):

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
ios/chrome/browser/ios_chrome_io_thread_unittest.mm:42: class
TestURLFetcherDelegate : public net::URLFetcherDelegate {
This should be in an anonymous namespace (I'd put the whole file in one, but
only this needs to be in one)

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
ios/chrome/browser/ios_chrome_io_thread_unittest.mm:67:
static_cast<web::WebThreadDelegate*>(ios_chrome_io_thread.get());
This is weird.  I guess it's needed because IOSChromeIOThread override public
methods with private ones?

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
ios/chrome/browser/ios_chrome_io_thread_unittest.mm:77:
base::RunLoop().RunUntilIdle();
What guarantee do we have that the fetcher wont complete when we do this? 
Should we do URLRequestFailedJob::AddUrlHandler() and
URLRequestFailedJob::GetMockHttpUrl(ERR_IO_PENDING)?

[mef, 2016-07-27 18:03:56]

The CQ bit was checked by mef@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 18:04:21]

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

[mef, 2016-07-27 18:04:27]

Thanks!

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
File ios/chrome/browser/ios_chrome_io_thread.mm (right):

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
ios/chrome/browser/ios_chrome_io_thread.mm:234: if (shutting_down_)
On 2016/07/27 17:23:45, mmenke wrote:
> Could null out io_thread_ instead, don't think it really matters, either way.

Done.

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
File ios/chrome/browser/ios_chrome_io_thread_unittest.mm (right):

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
ios/chrome/browser/ios_chrome_io_thread_unittest.mm:42: class
TestURLFetcherDelegate : public net::URLFetcherDelegate {
On 2016/07/27 17:23:45, mmenke wrote:
> This should be in an anonymous namespace (I'd put the whole file in one, but
> only this needs to be in one)

Done.

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
ios/chrome/browser/ios_chrome_io_thread_unittest.mm:67:
static_cast<web::WebThreadDelegate*>(ios_chrome_io_thread.get());
On 2016/07/27 17:23:45, mmenke wrote:
> This is weird.  I guess it's needed because IOSChromeIOThread override public
> methods with private ones?

Exactly.

https://codereview.chromium.org/2170103002/diff/80001/ios/chrome/browser/ios_...
ios/chrome/browser/ios_chrome_io_thread_unittest.mm:77:
base::RunLoop().RunUntilIdle();
On 2016/07/27 17:23:46, mmenke wrote:
> What guarantee do we have that the fetcher wont complete when we do this? 
> Should we do URLRequestFailedJob::AddUrlHandler() and
> URLRequestFailedJob::GetMockHttpUrl(ERR_IO_PENDING)?

Done.

[mmenke, 2016-07-27 18:10:43]

https://codereview.chromium.org/2170103002/diff/100001/ios/chrome/browser/ios...
File ios/chrome/browser/ios_chrome_io_thread_unittest.mm (right):

https://codereview.chromium.org/2170103002/diff/100001/ios/chrome/browser/ios...
ios/chrome/browser/ios_chrome_io_thread_unittest.mm:42:
io_thread_(web::WebThread::IO, &loop_){};
Semi-colon not needed, need space before open brace.

https://codereview.chromium.org/2170103002/diff/100001/ios/chrome/browser/ios...
ios/chrome/browser/ios_chrome_io_thread_unittest.mm:48:
net::URLRequestFailedJob::AddUrlHandler();
Oops...Should call net::URLRequestFilter::ClearAllFilters (Or whatever it's
called) in TearDown.

Or move them to the constructor/destructor. I don't think they need to be in
SetUp.TearDown, both of which, I'd just remove.

[mef, 2016-07-27 18:26:52]

Thanks again!

https://codereview.chromium.org/2170103002/diff/100001/ios/chrome/browser/ios...
File ios/chrome/browser/ios_chrome_io_thread_unittest.mm (right):

https://codereview.chromium.org/2170103002/diff/100001/ios/chrome/browser/ios...
ios/chrome/browser/ios_chrome_io_thread_unittest.mm:42:
io_thread_(web::WebThread::IO, &loop_){};
On 2016/07/27 18:10:43, mmenke wrote:
> Semi-colon not needed, need space before open brace.

Done.

https://codereview.chromium.org/2170103002/diff/100001/ios/chrome/browser/ios...
ios/chrome/browser/ios_chrome_io_thread_unittest.mm:48:
net::URLRequestFailedJob::AddUrlHandler();
On 2016/07/27 18:10:43, mmenke wrote:
> Oops...Should call net::URLRequestFilter::ClearAllFilters (Or whatever it's
> called) in TearDown.
> 
> Or move them to the constructor/destructor. I don't think they need to be in
> SetUp.TearDown, both of which, I'd just remove.

Done.

[mef, 2016-07-27 18:43:24]

The CQ bit was checked by mef@chromium.org

[mef, 2016-07-27 18:43:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
justincohen@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2170103002/#ps120001
(title: "Few more comments.")

[commit-bot: I haz the power, 2016-07-27 18:43:53]

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, 2016-07-27 19:39:15]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-07-27 19:40:53]

Description was changed from

==========
Notify SystemURLRequestContextGetter before shutdown on iOS.

BUG=418060
==========

to

==========
Notify SystemURLRequestContextGetter before shutdown on iOS.

BUG=418060

Committed: https://crrev.com/4bf02abe2596e899205a914e63cad289992cf7f7
Cr-Commit-Position: refs/heads/master@{#408215}
==========

[commit-bot: I haz the power, 2016-07-27 19:40:55]

Patchset 7 (id:??) landed as
https://crrev.com/4bf02abe2596e899205a914e63cad289992cf7f7
Cr-Commit-Position: refs/heads/master@{#408215}