Workaround DwmGetCompositionTimingInfo giving bad qpcRefreshPeriod

Workaround DwmGetCompositionTimingInfo giving bad qpcRefreshPeriod

On at least one machine Chrome hits a problem where DwmGetCompositionTimingInfo
says that qpcRefreshPeriod is 60. This implies a refresh rate of ~34,000 fps.
Chrome dutifully tries to hit this frame rate leading to excessive CPU usage.

This change adds a couple of checks to detect bad values and fix them. In
parallel this bug is being reported.

R=piman@chromium.org
BUG=486226
Committed: https://crrev.com/66c949d2f5478a20be46316c338174d5c3ad8735
Cr-Commit-Position: refs/heads/master@{#329983}

[brucedawson, 2015-05-14 22:13:10]

brucedawson@chromium.org changed reviewers:
+ kbr@chromium.org
- piman@chromium.org

[brucedawson, 2015-05-14 22:34:16]

This is a hack fix for a driver problem. If I've done my work properly it should
only trigger on machines where DwmGetCompositionTimingInfo is returning bad
information (a qpcRefreshPeriod of 60 was seen on one machine).

I put the 'fix' in WinVSyncProvider::GetVSyncParameters because it has access to
the rateRefresh information. This allows a second sanity check (making sure that
rateRefresh and qpcRefreshPeriod are vaguely in sync) and it allows me to
calculate what should be the correct value from rateRefresh.

I individually tested both detection paths and verified that they caught the
issue and set interval to 16,666. I'm doing further testing to make sure that
they *don't* trigger when all is well.

Possible risks: if there are drivers where qpcRefreshPeriod is good but
rateRefresh is bad then this could make things worse. That risk could be avoided
by only having the first check.

[Ken Russell (switch to Gerrit), 2015-05-14 22:38:20]

kbr@chromium.org changed reviewers:
+ jbauman@chromium.org, piman@chromium.org

[Ken Russell (switch to Gerrit), 2015-05-14 22:38:20]

This is great, but I'd like to defer review to jbauman or piman.

[Ken Russell (switch to Gerrit), 2015-05-14 22:38:46]

Of course, if Brian or Sunny would like to review this that's great too.

[jbauman, 2015-05-14 22:42:39]

lgtm

https://codereview.chromium.org/1141013004/diff/1/ui/gl/gl_surface_win.cc
File ui/gl/gl_surface_win.cc (right):

https://codereview.chromium.org/1141013004/diff/1/ui/gl/gl_surface_win.cc#new...
ui/gl/gl_surface_win.cc:82: base::TimeDelta interval2;
Nit, maybe call this "backup_interval".

[brucedawson, 2015-05-14 22:51:57]

https://codereview.chromium.org/1141013004/diff/1/ui/gl/gl_surface_win.cc
File ui/gl/gl_surface_win.cc (right):

https://codereview.chromium.org/1141013004/diff/1/ui/gl/gl_surface_win.cc#new...
ui/gl/gl_surface_win.cc:82: base::TimeDelta interval2;
On 2015/05/14 22:42:39, jbauman wrote:
> Nit, maybe call this "backup_interval".

How about 'rate_interval'. That gives more of a reminder of where it came from.

[Ken Russell (switch to Gerrit), 2015-05-14 23:02:20]

lgtm when jbauman's comments are addressed

[sunnyps, 2015-05-14 23:05:13]

sunnyps@chromium.org changed reviewers:
+ sunnyps@chromium.org

[sunnyps, 2015-05-14 23:05:14]

LGTM.

https://codereview.chromium.org/1141013004/diff/1/ui/gl/gl_surface_win.cc
File ui/gl/gl_surface_win.cc (right):

https://codereview.chromium.org/1141013004/diff/1/ui/gl/gl_surface_win.cc#new...
ui/gl/gl_surface_win.cc:82: base::TimeDelta interval2;
On 2015/05/14 22:51:57, brucedawson wrote:
> On 2015/05/14 22:42:39, jbauman wrote:
> > Nit, maybe call this "backup_interval".
> 
> How about 'rate_interval'. That gives more of a reminder of where it came
from.

I don't care much about the name but I like low_res_interval or something like
that.

[jbauman, 2015-05-14 23:12:42]

On 2015/05/14 22:51:57, brucedawson wrote:
> https://codereview.chromium.org/1141013004/diff/1/ui/gl/gl_surface_win.cc
> File ui/gl/gl_surface_win.cc (right):
> 
>
https://codereview.chromium.org/1141013004/diff/1/ui/gl/gl_surface_win.cc#new...
> ui/gl/gl_surface_win.cc:82: base::TimeDelta interval2;
> On 2015/05/14 22:42:39, jbauman wrote:
> > Nit, maybe call this "backup_interval".
> 
> How about 'rate_interval'. That gives more of a reminder of where it came
from.

That would work for me.

[brucedawson, 2015-05-14 23:15:05]

I renamed interval2 to rate_interval and tweaked the comment before its
declaration.

I also stepped through the code with various qpcRefreshPeriod values so I'm now
confident in the code.

Committing in a few minutes.

[brucedawson, 2015-05-14 23:20:34]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2015-05-14 23:20:35]

The patchset sent to the CQ was uploaded after l-g-t-m from kbr@chromium.org,
sunnyps@chromium.org, jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/1141013004/#ps20001
(title: "Rename interval2 to rate_interval")

[commit-bot: I haz the power, 2015-05-14 23:21:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1141013004/20001

[commit-bot: I haz the power, 2015-05-15 00:07:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-15 00:08:38]

Patchset 2 (id:??) landed as
https://crrev.com/66c949d2f5478a20be46316c338174d5c3ad8735
Cr-Commit-Position: refs/heads/master@{#329983}