Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(407)

Issue 541203002: fix for high resolution timer on windows. (Closed)

Created:
6 years, 3 months ago by cpu_(ooo_6.6-7.5)
Modified:
6 years, 3 months ago
Reviewers:
jamesr, Nico
CC:
chromium-reviews, erikwright+watch_chromium.org, sadrul
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

fix for high resolution timer on windows. The CLs here https://codereview.chromium.org/489793003 and here https://codereview.chromium.org/395913006 In isolation look correct but taken together cause a overflow or underflow bug. Basically the message loop was calling Time::ActivateHighResolutionTimer(false) all the time (or very often) so the g_high_res_timer_count was underflowing or overflowing. Now messageloop only calls ActivateHighResolutionTimer in a balanced way. This can make the base_unittests fail as well with --gtest_filter=MessageLoopTest.HighResolutionTimer BUG=153139 TEST=included, see bug for manual testing. Committed: https://crrev.com/3365a510b17169457292bdb6144cc8b95fb7ea34 Cr-Commit-Position: refs/heads/master@{#293434}

Patch Set 1 #

Total comments: 2

Patch Set 2 : nits fixes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -8 lines) Patch
M base/message_loop/message_loop.cc View 1 chunk +5 lines, -2 lines 0 comments Download
M base/message_loop/message_loop_unittest.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M base/time/time_win.cc View 1 2 chunks +9 lines, -6 lines 0 comments Download

Messages

Total messages: 8 (2 generated)
cpu_(ooo_6.6-7.5)
ptal
6 years, 3 months ago (2014-09-04 22:53:39 UTC) #2
jamesr
lgtm https://codereview.chromium.org/541203002/diff/1/base/time/time_win.cc File base/time/time_win.cc (right): https://codereview.chromium.org/541203002/diff/1/base/time/time_win.cc#newcode95 base/time/time_win.cc:95: unsigned int g_high_res_timer_count = 0; uint32_t 'unsigned int' ...
6 years, 3 months ago (2014-09-04 22:56:03 UTC) #3
Nico
lgtm
6 years, 3 months ago (2014-09-04 23:07:56 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cpu@chromium.org/541203002/20001
6 years, 3 months ago (2014-09-04 23:24:06 UTC) #6
commit-bot: I haz the power
Committed patchset #2 (id:20001) as aea6dacaebfaa582e88ed2703b65b2c44ed6c22d
6 years, 3 months ago (2014-09-05 04:30:50 UTC) #7
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:36:40 UTC) #8
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/3365a510b17169457292bdb6144cc8b95fb7ea34
Cr-Commit-Position: refs/heads/master@{#293434}

Powered by Google App Engine
This is Rietveld 408576698