New issue
Advanced search Search tips

Issue 742547 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

DCHECK_CURRENTLY_ON misleading in some browsing_data tests

Project Member Reported by btolsch@chromium.org, Jul 13 2017

Issue description

DCHECK_CURRENTLY_ON will succeed for any thread label when run with TestBrowserThreadBundle with no additional threads because TestBrowserThreadBundle runs everything in one thread and MessageLoop.

This applies to the following test files:
 - c/b/browsing_data/bookmark_counter_unittest.cc
 - c/b/browsing_data/browsing_data_channel_id_helper_unittest.cc
 - c/b/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
 - c/b/browsing_data/site_data_counting_helper_unittest.cc

 
Cc: msramek@chromium.org
Status: Available (was: Untriaged)
Owner: dullweber@chromium.org
Status: Started (was: Available)
I added some of these, so I can fix this. I guess the only option is to remove the dchecks because we can't test for the right thread anyway?
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1954999d8d84f4307e1ff25efddae6021d5a84b7

commit 1954999d8d84f4307e1ff25efddae6021d5a84b7
Author: Christian Dullweber <dullweber@chromium.org>
Date: Wed Jul 19 08:08:12 2017

Remove misleading DCHECK_CURRENTLY_ON from unittests

DCHECK_CURRENTLY_ON will succeed for any thread label when run with
TestBrowserThreadBundle with no additional threads because
TestBrowserThreadBundle runs everything in one thread and MessageLoop.
Therefor DCHECK_CURRENTLY_ON is removed from browsing_data unittests.

On top of that, WaitForTasksOnIOThread() in site data counter is 
improved to avoid a possible race condition if Done() is called 
before Wait().

Bug:  742547 
Change-Id: I0f9fd0533fbffd50bf2bb6be832dbc8f1302e496
Reviewed-on: https://chromium-review.googlesource.com/575048
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487785}
[modify] https://crrev.com/1954999d8d84f4307e1ff25efddae6021d5a84b7/chrome/browser/browsing_data/bookmark_counter_unittest.cc
[modify] https://crrev.com/1954999d8d84f4307e1ff25efddae6021d5a84b7/chrome/browser/browsing_data/browsing_data_channel_id_helper_unittest.cc
[modify] https://crrev.com/1954999d8d84f4307e1ff25efddae6021d5a84b7/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
[modify] https://crrev.com/1954999d8d84f4307e1ff25efddae6021d5a84b7/chrome/browser/browsing_data/site_data_counting_helper_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment