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

Issue 2549623006: Fix TestBrowserThread destruction order in CrOS tests. (Closed)

Created:
4 years ago by gab
Modified:
4 years ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, asvitkine+watch_chromium.org, yamaguchi+watch_chromium.org, oka+watch_chromium.org, rginda+watch_chromium.org, oshima+watch_chromium.org, fukino+watch_chromium.org, stevenjb+watch_chromium.org, davemoore+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix TestBrowserThread destruction order in CrOS tests. This is a prereq to https://codereview.chromium.org/2464233002 which will from now on reset BrowserThread globals associated with destroyed BrowserThreadImpls and thus makes DCHECK_CURRENTLY_ON fail after it was. BUG=653916 Committed: https://crrev.com/d4277b0cc41219c665fd3133ad72ce9b8039161b Cr-Commit-Position: refs/heads/master@{#436154}

Patch Set 1 #

Total comments: 3

Patch Set 2 : +comment on Leaky #

Messages

Total messages: 22 (12 generated)
gab
Easy review :-) @oshima for chromeos/ @sque for leak_detector/ @isherman for easy_unlock_* Thanks! Gab https://codereview.chromium.org/2549623006/diff/1/chrome/browser/metrics/leak_detector/leak_detector_controller_unittest.cc ...
4 years ago (2016-12-02 19:23:51 UTC) #4
gab
On 2016/12/02 19:23:51, gab wrote: > Easy review :-) > > @oshima for chromeos/ > ...
4 years ago (2016-12-02 19:26:27 UTC) #7
Simon Que
lgtm for leak_detector_controller_unittest.cc
4 years ago (2016-12-02 19:28:23 UTC) #8
oshima
lgtm with a nit https://codereview.chromium.org/2549623006/diff/1/chrome/browser/metrics/leak_detector/leak_detector_controller_unittest.cc File chrome/browser/metrics/leak_detector/leak_detector_controller_unittest.cc (right): https://codereview.chromium.org/2549623006/diff/1/chrome/browser/metrics/leak_detector/leak_detector_controller_unittest.cc#newcode62 chrome/browser/metrics/leak_detector/leak_detector_controller_unittest.cc:62: base::LazyInstance<TestLeakDetectorController>::Leaky g_instance = On 2016/12/02 ...
4 years ago (2016-12-02 20:18:12 UTC) #11
oshima
lgtm with a nit
4 years ago (2016-12-02 20:18:14 UTC) #12
Ilya Sherman
easy_unlock lgtm
4 years ago (2016-12-02 23:53:35 UTC) #13
gab
Thanks, done. https://codereview.chromium.org/2549623006/diff/1/chrome/browser/metrics/leak_detector/leak_detector_controller_unittest.cc File chrome/browser/metrics/leak_detector/leak_detector_controller_unittest.cc (right): https://codereview.chromium.org/2549623006/diff/1/chrome/browser/metrics/leak_detector/leak_detector_controller_unittest.cc#newcode62 chrome/browser/metrics/leak_detector/leak_detector_controller_unittest.cc:62: base::LazyInstance<TestLeakDetectorController>::Leaky g_instance = On 2016/12/02 20:18:12, oshima ...
4 years ago (2016-12-03 03:02:27 UTC) #14
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/2549623006/20001
4 years ago (2016-12-03 03:03:01 UTC) #17
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years ago (2016-12-03 03:15:55 UTC) #20
commit-bot: I haz the power
4 years ago (2016-12-03 03:17:29 UTC) #22
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/d4277b0cc41219c665fd3133ad72ce9b8039161b
Cr-Commit-Position: refs/heads/master@{#436154}

Powered by Google App Engine
This is Rietveld 408576698