More aggressive IO asserts in content_browsertests.

More aggressive IO asserts in content_browsertests.

Since browser test's main thread corresponds roughly to the product's UI
thread, it should behave in the same wrt base::ThreadRestrictions -
after this CL the body of browser tests derived from
content::ContentBrowserTest is not allowed to do file IO.

Motivation: The more aggressive IO asserts would have caught
https://crbug.com/626972 earlier.  Hopefully the more aggressive
asserting will also prevent future regressions in this area.

BUG=631145
Committed: https://crrev.com/7947ccd5754451960f0bf18f3b5401dc5ffeedb8
Cr-Commit-Position: refs/heads/master@{#408496}

[Łukasz Anforowicz, 2016-07-26 00:23:51]

Description was changed from

==========
More aggressive IO asserts in content_browsertests.

BUG=
==========

to

==========
More aggressive IO asserts in content_browsertests.

Since browser test's main thread corresponds roughly to the product's UI
thread, it should behave in the same wrt base::ThreadRestrictions -
after this CL the body of browser tests derived from
content::ContentBrowserTest is not allowed to do file IO.

Motivation: The more aggressive IO asserts would have caught
https://crbug.com/626972 earlier.  Hopefully the more aggressive
asserting will also prevent future regressions in this area.

BUG=631145
==========

[Łukasz Anforowicz, 2016-07-26 22:28:59]

lukasza@chromium.org changed reviewers:
+ jam@chromium.org

[Łukasz Anforowicz, 2016-07-26 22:28:59]

jam@, can you take a look please?

The main change in this CL is to call
base::ThreadRestrictions::SetIOAllowed(false);
before executing test body of a browser test derived from
content::ContentBrowserTest class.  This is motivated by desire to catch issues
like https://crbug.com/626972 in a systematic way (i.e. without having to opt-in
individual test suites like in an earlier/obsolete patchset at
https://codereview.chromium.org/2168403003/patch/1/10001).

The other changes in this CL are a consequence of disallowing IO from the test
body of //content browser tests - parts of test code that need to do IO had to
be sprinkled with base::ThreadRestrictions::ScopedAllowIO.  I think this is fine
despite warnings in PRESUBMIT.py - the IO asserts are intended to catch IO done
from *product* code, not from *test* code.

Related CLs:

- https://codereview.chromium.org/2168403003/ (getting rid of IO on UI thread
trigerred by SavePackage;  this needs to land before the CL under review, but CR
might be delayed because rdsmith@ and asanka@ are currently OOO)

- https://codereview.chromium.org/2178683002/ (attempt to add similar
verification to browser tests under //chrome)

[jam, 2016-07-26 23:52:16]

lgtm

good catch, i agree slightly more work in browser tests is worth the effort to
catch incorrect IO in our tests.

https://codereview.chromium.org/2175933002/diff/80001/content/browser/accessi...
File content/browser/accessibility/dump_accessibility_browsertest_base.cc
(right):

https://codereview.chromium.org/2175933002/diff/80001/content/browser/accessi...
content/browser/accessibility/dump_accessibility_browsertest_base.cc:211: }
nit: seems more readable to keep this thread restriction scope till line 232

[Łukasz Anforowicz, 2016-07-27 16:18:14]

Thanks for the review.

https://codereview.chromium.org/2175933002/diff/80001/content/browser/accessi...
File content/browser/accessibility/dump_accessibility_browsertest_base.cc
(right):

https://codereview.chromium.org/2175933002/diff/80001/content/browser/accessi...
content/browser/accessibility/dump_accessibility_browsertest_base.cc:211: }
On 2016/07/26 23:52:16, jam wrote:
> nit: seems more readable to keep this thread restriction scope till line 232

Done.

[Łukasz Anforowicz, 2016-07-27 16:18:42]

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

[commit-bot: I haz the power, 2016-07-27 16:18:59]

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 17:22:00]

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

[commit-bot: I haz the power, 2016-07-27 17:22:01]

Dry run: This issue passed the CQ dry run.

[Łukasz Anforowicz, 2016-07-28 21:06:29]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2016-07-28 21:06:29]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2175933002/#ps100001
(title: "Use 1 less instance of ScopedAllowIO in
DumpAccessibilityTestBase::RunTestForPlatform.")

[commit-bot: I haz the power, 2016-07-28 21:06:48]

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-28 21:56:39]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-07-28 21:58:51]

Description was changed from

==========
More aggressive IO asserts in content_browsertests.

Since browser test's main thread corresponds roughly to the product's UI
thread, it should behave in the same wrt base::ThreadRestrictions -
after this CL the body of browser tests derived from
content::ContentBrowserTest is not allowed to do file IO.

Motivation: The more aggressive IO asserts would have caught
https://crbug.com/626972 earlier.  Hopefully the more aggressive
asserting will also prevent future regressions in this area.

BUG=631145
==========

to

==========
More aggressive IO asserts in content_browsertests.

Since browser test's main thread corresponds roughly to the product's UI
thread, it should behave in the same wrt base::ThreadRestrictions -
after this CL the body of browser tests derived from
content::ContentBrowserTest is not allowed to do file IO.

Motivation: The more aggressive IO asserts would have caught
https://crbug.com/626972 earlier.  Hopefully the more aggressive
asserting will also prevent future regressions in this area.

BUG=631145

Committed: https://crrev.com/7947ccd5754451960f0bf18f3b5401dc5ffeedb8
Cr-Commit-Position: refs/heads/master@{#408496}
==========

[commit-bot: I haz the power, 2016-07-28 21:58:52]

Patchset 6 (id:??) landed as
https://crrev.com/7947ccd5754451960f0bf18f3b5401dc5ffeedb8
Cr-Commit-Position: refs/heads/master@{#408496}