New issue
Advanced search Search tips

Issue 865802 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 27
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Three BrowsingDataRemoverBrowserTests failing on various platforms

Project Member Reported by alex...@chromium.org, Jul 20

Issue description

The following three tests have recently started failing on various bots:
BrowsingDataRemoverBrowserTestP.SessionStorageDeletion/1
BrowsingDataRemoverBrowserTestP.SessionStorageDeletion/0
BrowsingDataRemoverBrowserTest.StorageRemovedFromDisk

Sample failures:
../../chrome/browser/browsing_data/browsing_data_remover_browsertest.cc:859: Failure
Expected equality of these values:
  0
  found
    Which is: 1

and 

../../chrome/browser/browsing_data/browsing_data_remover_browsertest.cc:737: Failure
Value of: HasDataForType(type)
  Actual: true
Expected: false


Some of the builds in which the failures started:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests/71136
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win7%20%2832%29%20Tests/35832
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win%207%20Tests%20x64%20%281%29/40972
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-rel/10896
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/mac-cocoa-rel/470

Intersecting blamelists, the range seems to be r576611-r576606.  
The storage-related changes here are r576611 (Add Mac support for BrowserDMTokenStorage) and r576609 (Revert "[SessionStorageS13N] Enabling mojo SessionStorage by default").  I'm suspecting r576609, since the failures are not Mac-specific, but I'm hesitant to revert a revert.

dmurph@ and georgesak@ - can you take a look?
 
dmurph@ confirmed offline that these tests should be disabled and that the revert in r576609 needs to stay in for the branch.  I'll go ahead and disabled the tests.
Cc: dullweber@chromium.org
Note that when I was disabling SessionStorageDeletion, I noticed there was both a SessionStorageDeletion and a DISABLED_SessionStorageDeletion.  Someone might want to rename one of those tests, as I'm not sure how they're different.
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 20

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

commit a1065a66bc7298be5bcc2124c451f90eb2a8148b
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Fri Jul 20 01:15:06 2018

Disable three failing BrowsingDataRemoverBrowserTests.

Bug:  865802 
Change-Id: I8c6a6eb45d29c9cdd1aec9557baebd3bd58cb332
Tbr: dmurph@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1144521
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576746}
[modify] https://crrev.com/a1065a66bc7298be5bcc2124c451f90eb2a8148b/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc

The old DISABLED_SessionStorageDeletion is the test that should be enabled when Session Storage deletion works correctly and is integrated with CookiesTreeModel and the SiteDataCounter. 
The SessionStorageDeletion test that was disabled is a subset of the other test that only tests the behavior on the web. 

The CL above has been relanded: https://crrev.com/c/1145201
Can we enable the tests again?
Labels: -Sheriff-Chromium
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 27

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

commit a8117de4347200bd9e5fbe6eebf7fd024f5b170f
Author: Christian Dullweber <dullweber@chromium.org>
Date: Fri Jul 27 07:45:03 2018

Enable BrowsingDataRemoverTests

BrowsingDataRemoverBrowserTestP.SessionStorageDeletion and
BrowsingDataRemoverBrowserTest.StorageRemovedFromDisk were flaky when
MojoSessionStorage was temporarily disabled.

This CL enables the tests again and forces the kMojoSessionStorage flag
to be enabled during the test.

Bug:  865802 
Change-Id: I1f3b0b2bad792089f624a33271600ecf23875b29
Reviewed-on: https://chromium-review.googlesource.com/1150532
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578560}
[modify] https://crrev.com/a8117de4347200bd9e5fbe6eebf7fd024f5b170f/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc

Cc: -dullweber@chromium.org dmu...@chromium.org
Owner: dullweber@chromium.org
Status: Fixed (was: Assigned)

Sign in to add a comment