Ensure compositing test preference overrides are reset between tests

Ensure compositing test preference overrides are reset between tests

Layouttests in the compositing/ directory override the default
preferences to enable accelerated 2d canvas and enable mock
scrollbars[1]. These preferences were not reset properly so tests
following the compositing test could run with the compositing overrides.

This patch forces preferences to be reset before every test.

This flakiness was observable on the bots where tests would flakily
fail with incorrect scrollbars:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=compositing%2Flayers-inside-overflow-scroll.html&showExpectations=true

To reproduce this bug reliably, run:
run-webkit-tests compositing/animation/state-at-end-event-transform-layer.html fast/canvas/canvas-render-layer.html --additional-driver-flag=--enable-slimming-paint-v2 --child-processes=1
which will fail. Running the tests individually passes.

[1] https://cs.chromium.org/chromium/src/content/shell/browser/layout_test/blink_test_controller.cc?l=369

BUG=633707, 627798, 634685
Committed: https://crrev.com/a5191dca2b309396a7743816917f19d037ebc39a
Cr-Commit-Position: refs/heads/master@{#410485}

[pdr., 2016-08-04 05:29:38]

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

[commit-bot: I haz the power, 2016-08-04 05:29:49]

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

[pdr., 2016-08-04 06:28:41]

pdr@chromium.org changed reviewers:
+ chrishtr@chromium.org, mkwst@chromium.org, wangxianzhu@chromium.org

[pdr., 2016-08-04 06:28:49]

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

[commit-bot: I haz the power, 2016-08-04 06:29:26]

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-08-04 07:16:38]

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

[commit-bot: I haz the power, 2016-08-04 07:16:39]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Mike West, 2016-08-04 11:25:03]

LGTM, thanks for the descriptive CL description.

[chrishtr, 2016-08-04 15:30:07]

lgtm

[Xianzhu, 2016-08-04 15:53:26]

https://codereview.chromium.org/2208603004/diff/40001/content/shell/browser/l...
File content/shell/browser/layout_test/blink_test_controller.cc (right):

https://codereview.chromium.org/2208603004/diff/40001/content/shell/browser/l...
content/shell/browser/layout_test/blink_test_controller.cc:315: if
(is_compositing_test_) {
Can we just omit this condition, to ensure the prefs are reset whatever prefs
are overridden?

Also a question: when is BlinkTestController::OverrideWebkitPrefs() called? I
saw prefs are override in it. Should it be called for every test?

[Xianzhu, 2016-08-04 16:04:52]

https://codereview.chromium.org/2208603004/diff/40001/content/shell/browser/l...
File content/shell/browser/layout_test/blink_test_controller.cc (right):

https://codereview.chromium.org/2208603004/diff/40001/content/shell/browser/l...
content/shell/browser/layout_test/blink_test_controller.cc:321:
render_view_host->UpdateWebkitPreferences(default_prefs_);
Would it work to just change the above call to the following and omit the added
code line 315-320?

  render_view_host->OnWebkitPreferencesChanged();
  default_prefs_ = render_view_host->GetWebkitPreferences()

https://codereview.chromium.org/2208603004/diff/40001/content/shell/browser/l...
content/shell/browser/layout_test/blink_test_controller.cc:350:
is_compositing_test_ = false;
1. Is the above code necessary as we always update preferences before starting a
layout test?

2. If this is necessary, should we put is_compositing_test_ = false at the
beginning of this block?

[pdr., 2016-08-08 21:23:23]

Description was changed from

==========
Ensure compositing test preference overrides are reset after tests

Layouttests in the compositing/ directory override the default
preferences to enable accelerated 2d canvas and enable mock
scrollbars[1]. These preferences were not reset properly so tests
following the compositing test could run with the compositing overrides.

This patch forces preferences to be reset after every compositing test
run.

This flakiness was observable on the bots where tests would flakily
fail with incorrect scrollbars:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=com...

To reproduce this bug reliably, run:
run-webkit-tests compositing/animation/state-at-end-event-transform-layer.html
fast/canvas/canvas-render-layer.html
--additional-driver-flag=--enable-slimming-paint-v2 --child-processes=1
which will fail. Running the tests individually passes.

[1]
https://cs.chromium.org/chromium/src/content/shell/browser/layout_test/blink_...

BUG=633707,627798
==========

to

==========
Ensure compositing test preference overrides are reset after tests

Layouttests in the compositing/ directory override the default
preferences to enable accelerated 2d canvas and enable mock
scrollbars[1]. These preferences were not reset properly so tests
following the compositing test could run with the compositing overrides.

This patch forces preferences to be reset before every test.

This flakiness was observable on the bots where tests would flakily
fail with incorrect scrollbars:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=com...

To reproduce this bug reliably, run:
run-webkit-tests compositing/animation/state-at-end-event-transform-layer.html
fast/canvas/canvas-render-layer.html
--additional-driver-flag=--enable-slimming-paint-v2 --child-processes=1
which will fail. Running the tests individually passes.

[1]
https://cs.chromium.org/chromium/src/content/shell/browser/layout_test/blink_...

BUG=633707,627798,634685
==========

[pdr., 2016-08-08 21:23:37]

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

[commit-bot: I haz the power, 2016-08-08 21:23:53]

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

[pdr., 2016-08-08 21:29:40]

https://codereview.chromium.org/2208603004/diff/40001/content/shell/browser/l...
File content/shell/browser/layout_test/blink_test_controller.cc (right):

https://codereview.chromium.org/2208603004/diff/40001/content/shell/browser/l...
content/shell/browser/layout_test/blink_test_controller.cc:315: if
(is_compositing_test_) {
On 2016/08/04 at 15:53:26, Xianzhu wrote:
> Can we just omit this condition, to ensure the prefs are reset whatever prefs
are overridden?

Good idea. I thought that might be too slow but found we actually did it before
https://crrev.com/d4c23ca618abaeeeba8b7f707bb2b1cb4460d642.

> 
> Also a question: when is BlinkTestController::OverrideWebkitPrefs() called? I
saw prefs are override in it. Should it be called for every test?

The !main_window_ codepath seems to be hit for a process's first test run which
will call OverrideWebkitPrefs through the following:
RenderViewHostImpl::GetWebkitPreferences
RenderViewHostImpl::OnWebkitPreferencesChanged
RenderViewHostImpl::ComputeWebkitPrefs
LayoutTestContentBrowserClient::OverrideWebkitPrefs

For subsequent test runs (main_window_ != null), we'll now explicitly override
the preferences every time with the latest patch.

[Xianzhu, 2016-08-08 21:50:45]

lgtm

[pdr., 2016-08-08 21:52:59]

Description was changed from

==========
Ensure compositing test preference overrides are reset after tests

Layouttests in the compositing/ directory override the default
preferences to enable accelerated 2d canvas and enable mock
scrollbars[1]. These preferences were not reset properly so tests
following the compositing test could run with the compositing overrides.

This patch forces preferences to be reset before every test.

This flakiness was observable on the bots where tests would flakily
fail with incorrect scrollbars:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=com...

To reproduce this bug reliably, run:
run-webkit-tests compositing/animation/state-at-end-event-transform-layer.html
fast/canvas/canvas-render-layer.html
--additional-driver-flag=--enable-slimming-paint-v2 --child-processes=1
which will fail. Running the tests individually passes.

[1]
https://cs.chromium.org/chromium/src/content/shell/browser/layout_test/blink_...

BUG=633707,627798,634685
==========

to

==========
Ensure compositing test preference overrides are reset between tests

Layouttests in the compositing/ directory override the default
preferences to enable accelerated 2d canvas and enable mock
scrollbars[1]. These preferences were not reset properly so tests
following the compositing test could run with the compositing overrides.

This patch forces preferences to be reset before every test.

This flakiness was observable on the bots where tests would flakily
fail with incorrect scrollbars:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=com...

To reproduce this bug reliably, run:
run-webkit-tests compositing/animation/state-at-end-event-transform-layer.html
fast/canvas/canvas-render-layer.html
--additional-driver-flag=--enable-slimming-paint-v2 --child-processes=1
which will fail. Running the tests individually passes.

[1]
https://cs.chromium.org/chromium/src/content/shell/browser/layout_test/blink_...

BUG=633707,627798,634685
==========

[commit-bot: I haz the power, 2016-08-08 22:44:24]

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

[commit-bot: I haz the power, 2016-08-08 22:44:25]

Dry run: This issue passed the CQ dry run.

[pdr., 2016-08-08 22:46:06]

The CQ bit was checked by pdr@chromium.org

[pdr., 2016-08-08 22:46:07]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org,
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/2208603004/#ps60001
(title: "Always reset preferences")

[commit-bot: I haz the power, 2016-08-08 22:46:45]

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-08-08 22:54:50]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-08 22:56:34]

Description was changed from

==========
Ensure compositing test preference overrides are reset between tests

Layouttests in the compositing/ directory override the default
preferences to enable accelerated 2d canvas and enable mock
scrollbars[1]. These preferences were not reset properly so tests
following the compositing test could run with the compositing overrides.

This patch forces preferences to be reset before every test.

This flakiness was observable on the bots where tests would flakily
fail with incorrect scrollbars:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=com...

To reproduce this bug reliably, run:
run-webkit-tests compositing/animation/state-at-end-event-transform-layer.html
fast/canvas/canvas-render-layer.html
--additional-driver-flag=--enable-slimming-paint-v2 --child-processes=1
which will fail. Running the tests individually passes.

[1]
https://cs.chromium.org/chromium/src/content/shell/browser/layout_test/blink_...

BUG=633707,627798,634685
==========

to

==========
Ensure compositing test preference overrides are reset between tests

Layouttests in the compositing/ directory override the default
preferences to enable accelerated 2d canvas and enable mock
scrollbars[1]. These preferences were not reset properly so tests
following the compositing test could run with the compositing overrides.

This patch forces preferences to be reset before every test.

This flakiness was observable on the bots where tests would flakily
fail with incorrect scrollbars:
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=com...

To reproduce this bug reliably, run:
run-webkit-tests compositing/animation/state-at-end-event-transform-layer.html
fast/canvas/canvas-render-layer.html
--additional-driver-flag=--enable-slimming-paint-v2 --child-processes=1
which will fail. Running the tests individually passes.

[1]
https://cs.chromium.org/chromium/src/content/shell/browser/layout_test/blink_...

BUG=633707,627798,634685

Committed: https://crrev.com/a5191dca2b309396a7743816917f19d037ebc39a
Cr-Commit-Position: refs/heads/master@{#410485}
==========

[commit-bot: I haz the power, 2016-08-08 22:56:36]

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