Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(193)

Issue 2571473002: Fix cases where RunLoop is created without a MessageLoop. (Closed)

Created:
2 years, 11 months ago by Alexander Semashko
Modified:
2 years, 11 months ago
Reviewers:
jam
CC:
chromium-reviews, vmpstr+watch_chromium.org, jam, Randy Smith (Not in Mondays), feature-media-reviews_chromium.org, darin-cc_chromium.org, loading-reviews_chromium.org, chirantan+watch_chromium.org, mmenke
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix cases where RunLoop is created without a MessageLoop. The tests in these cases can pass only if Quit is called before Run, or if Run is never called at all. If this is guaranteed to hold true, then RunLoop is not needed (such as in timer_unittest.cc). Other cases (where it is not guaranteed) are tests that can crash occasionally. BUG=668707 Committed: https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e Cr-Commit-Position: refs/heads/master@{#438126}

Patch Set 1 #

Patch Set 2 : Remove unused ScreenshotTester::run_loop_ that triggered the DCHECK on chromeos. #

Patch Set 3 : Add missing includes. #

Patch Set 4 : rebase #

Patch Set 5 : Fix chromeos tests. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+82 lines, -42 lines) Patch
M base/run_loop.cc View 1 chunk +1 line, -0 lines 0 comments Download
M base/timer/timer_unittest.cc View 3 chunks +54 lines, -29 lines 0 comments Download
M chrome/browser/chromeos/login/screenshot_testing/screenshot_tester.h View 1 2 2 chunks +3 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/login/screenshot_testing/screenshot_tester.cc View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/login/screenshot_testing/screenshot_testing_mixin.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/chromeos/login/signin/device_id_browsertest.cc View 1 2 3 4 4 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/chromeos/system/device_disabling_browsertest.cc View 1 2 3 4 4 chunks +5 lines, -3 lines 0 comments Download
M content/browser/loader/async_revalidation_manager_browsertest.cc View 3 chunks +12 lines, -3 lines 0 comments Download
M media/base/audio_renderer_mixer_input_unittest.cc View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 19 (14 generated)
Alexander Semashko
2 years, 11 months ago (2016-12-12 13:46:04 UTC) #2
jam
lgtm
2 years, 11 months ago (2016-12-12 17:58:07 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2571473002/80001
2 years, 11 months ago (2016-12-13 09:43:31 UTC) #14
commit-bot: I haz the power
Committed patchset #5 (id:80001)
2 years, 11 months ago (2016-12-13 11:35:20 UTC) #17
commit-bot: I haz the power
2 years, 11 months ago (2016-12-13 11:39:05 UTC) #19
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/68c9f104d87f31212b353d0fc20c3c57ec23125e
Cr-Commit-Position: refs/heads/master@{#438126}

Powered by Google App Engine
This is Rietveld 408576698