Fix ExternalViewportRectForPrioritizingTiles flakiness on windows

Fix ExternalViewportRectForPrioritizingTiles flakiness on Windows

2 problems here:
1. Setting the frame time to 0 does nothing, because LTHI will read from
gfx::FrameTime::Now() instead. Need to start at 1 millisecond.

2. Need to tick the frame timer after SetupDrawPropertiesAndUpdateTiles.

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

[hush (inactive), 2014-08-05 01:17:17]

The timestamp on windows (or double precision) is not as precise as on Android.
And we happen to depend on that in our tests. This fix ensures we don't depend
on the precision of gfx::FrameTime::Now() for the test.

active_layer_->UpdateTiles is called inside SetupDrawPropertiesAndUpdateTiles(),
so we should "tick the frame" after this call. Otherwise, when
active_layer_->UpdateTiles(NULL) is called in line 341, it may or may not be
ignored depending on how accurate the system timer is.

Actually, I think we should have ticked the frame time in other tests too, after
SetupDrawPropertiesAndUpdateTiles() calls.

[aelias_OOO_until_Jul13, 2014-08-05 02:45:19]

The idea that timer stuff can cause flakiness makes sense in the abstract, but I
still don't understand exactly what's happening here.  I don't see where in the
code any platform-dependent code is triggered.  TimeDelta and TimeTicks are
int64 on all platforms, and the test doesn't seem to rely on calling the system
timer given that it's faked to zero at the beginning of the test.

[hush (inactive), 2014-08-05 02:49:16]

On 2014/08/05 02:45:19, aelias wrote:
> The idea that timer stuff can cause flakiness makes sense in the abstract, but
I
> still don't understand exactly what's happening here.  I don't see where in
the
> code any platform-dependent code is triggered.  TimeDelta and TimeTicks are
> int64 on all platforms, and the test doesn't seem to rely on calling the
system
> timer given that it's faked to zero at the beginning of the test.

Yes, it is faked to zero. However, LTHI does not treat zero as a valid frame
time. It will use
the current system time instead. See here:
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/layer_tre...

[aelias_OOO_until_Jul13, 2014-08-05 02:53:40]

Ah, I see, that makes sense.  We should stop ever faking to zero then as it
doesn't do what's expected.  Let's start the fake time at 1 if we're setting it
at all.

Since you're on it, would you mind going through and cleaning up any/all the
tests with this problem?  Hopefully that can nip a few more flakes that aren't
getting attention.

[hush (inactive), 2014-08-05 02:58:23]

There are basically 2 problems I think:
1. Setting the frame time to 0 does nothing, because LTHI will read from
gfx::FrameTime::Now() instead. I think we can make it start at 1 millisecond
instead? Here is a dummy CL that shows the idea:
https://codereview.chromium.org/444533002/

2. Need to tick the frame timer after each call of
SetupDrawPropertiesAndUpdateTiles()

[hush (inactive), 2014-08-05 02:59:40]

On 2014/08/05 02:53:40, aelias wrote:
> Ah, I see, that makes sense.  We should stop ever faking to zero then as it
> doesn't do what's expected.  Let's start the fake time at 1 if we're setting
it
> at all.
> 
> Since you're on it, would you mind going through and cleaning up any/all the
> tests with this problem?  Hopefully that can nip a few more flakes that aren't
> getting attention.

Yeah. Exactly. I was typing up my previous reply before I saw your comment.
I will create another CL for cleaning up other unit tests then.

[aelias_OOO_until_Jul13, 2014-08-05 03:00:16]

On 2014/08/05 at 02:58:23, hush wrote:
> There are basically 2 problems I think:
> 1. Setting the frame time to 0 does nothing, because LTHI will read from
gfx::FrameTime::Now() instead. I think we can make it start at 1 millisecond
instead? Here is a dummy CL that shows the idea:
https://codereview.chromium.org/444533002/
> 
> 2. Need to tick the frame timer after each call of
SetupDrawPropertiesAndUpdateTiles()

Agreed.  I think you should do (1) first (on this and any other test that sets
frame time to 0).  And then likely you will get a consistent fail if you don't
do (2) as well (I would guess), so you will end up doing both.

[hush (inactive), 2014-08-05 03:16:38]

Updated the issue description and made the timer start at 1.

[aelias_OOO_until_Jul13, 2014-08-05 03:33:41]

lgtm

[hush (inactive), 2014-08-05 03:51:13]

The CQ bit was checked by hush@chromium.org

[commit-bot: I haz the power, 2014-08-05 03:53:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hush@chromium.org/444463004/20001

[commit-bot: I haz the power, 2014-08-05 06:55:58]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-08-05 08:24:52]

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

[commit-bot: I haz the power, 2014-08-05 08:24:53]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[danakj, 2014-08-05 13:16:48]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-08-05 13:17:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hush@chromium.org/444463004/20001

[commit-bot: I haz the power, 2014-08-05 14:01:35]

Change committed as 287536