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

Issue 3946003: Revert 63191 - 2nd try:... (Closed)

Created:
10 years, 2 months ago by Paweł Hajdan Jr.
Modified:
9 years, 7 months ago
Reviewers:
Mike Belshe
CC:
chromium-reviews, brettw-cc_chromium.org, pam+watch_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

Revert 63191 - 2nd try: Fix regression where high resolution timers could be activated even under battery power. Add unit test to protect chromium from developers like me in the future. The fix is a one-liner in hi_res_timer_manager_win.cc. The rest of the code change is the mechanics to enable the unit test. BUG=59528 TEST=HiResTimerManagerTest.ToggleOnOff Broke unit_tests on Vista and XP: [ RUN ] GeolocationNetworkProviderTest.GatewayAndWifiScans [2916:2376:1020/030222:2975321329:FATAL:network_location_provider.cc(51)] Check failed: cache_.size() == cache_times_.size(). Review URL: http://codereview.chromium.org/3889004 TBR=mbelshe@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=63200

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -103 lines) Patch
M app/app.gyp View 1 chunk +0 lines, -1 line 0 comments Download
M app/hi_res_timer_manager.h View 1 chunk +1 line, -4 lines 0 comments Download
M app/hi_res_timer_manager_posix.cc View 1 chunk +1 line, -1 line 0 comments Download
D app/hi_res_timer_manager_unittest.cc View 1 chunk +0 lines, -51 lines 0 comments Download
M app/hi_res_timer_manager_win.cc View 2 chunks +2 lines, -3 lines 0 comments Download
M base/message_loop.cc View 2 chunks +3 lines, -14 lines 0 comments Download
M base/time.h View 2 chunks +1 line, -10 lines 0 comments Download
M base/time_win.cc View 2 chunks +4 lines, -19 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Paweł Hajdan Jr.
10 years, 2 months ago (2010-10-20 10:42:19 UTC) #1
Paweł Hajdan Jr.
TBR
10 years, 2 months ago (2010-10-20 10:57:28 UTC) #2
Paweł Hajdan Jr.
It failed two or three times in a row on two different bots, but seems ...
10 years, 2 months ago (2010-10-20 11:59:20 UTC) #3
mbelshe
So, based on the build bot, looks like this revert was superfluous, right? In fact, ...
10 years, 2 months ago (2010-10-20 15:08:44 UTC) #4
Paweł Hajdan Jr.
On Wed, Oct 20, 2010 at 17:08, Mike Belshe <mbelshe@google.com> wrote: > So, based on ...
10 years, 2 months ago (2010-10-20 15:18:12 UTC) #5
mbelshe
10 years, 2 months ago (2010-10-20 15:20:30 UTC) #6
On Wed, Oct 20, 2010 at 8:17 AM, Paweł Hajdan, Jr.
<phajdan.jr@chromium.org>wrote:

> On Wed, Oct 20, 2010 at 17:08, Mike Belshe <mbelshe@google.com> wrote:
>
>> So, based on the build bot, looks like this revert was superfluous, right?
>
>
> I'm not sure. It cycled red two times in a row on two bots. I let it finish
> third cycle on one of the bots and it was also red I think. Then the third
> cycle on the second bot cycled green.
>
>
>> In fact, it looks like by the time this revert was done, the tree had
>> already cycled green on its own?
>>
>> Please confirm.
>>
>
> Yes, I think that was the case.
>
>
>> And for the record, I did have all green trybots, and I did watch the tree
>> after landing.
>>
>
> When I was looking at the codereview link, the windows trybot was red,
> compile failed (probably flaky). And the issue has been discussed a lot on
> irc before I committed the event.
>

The try bot was red on an obvious flakey, unrelated failure ...  Sorry that
I have to wait until 2am to checkin because flakiness keeps the tree closed
all day and that I didn't stick around for 3 cycles after the failure.  That
looked pretty likely to be just standard brokenness of our tree.


Mike

Powered by Google App Engine
This is Rietveld 408576698