New issue
Advanced search Search tips

Issue 879812 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

"BrowsingDataRemoverBrowserTest.MediaLicenseTimedDeletion" is flaky

Project Member Reported by chromium...@appspot.gserviceaccount.com, Sep 1

Issue description

"BrowsingDataRemoverBrowserTest.MediaLicenseTimedDeletion" is flaky.

This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label.

We have detected 5 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyQwsSBUZsYWtlIjhCcm93c2luZ0RhdGFSZW1vdmVyQnJvd3NlclRlc3QuTWVkaWFMaWNlbnNlVGltZWREZWxldGlvbgw.

Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
 
Cc: msramek@chromium.org xhw...@chromium.org
Labels: -Sheriff-Chromium
Owner: jrumm...@chromium.org
Status: Assigned (was: Untriaged)
Reverting r588090 in https://crrev.com/c/1201851 and adding original CL author. jrummel@, please take a look.
Looking into this. The original test runs fine locally (after reverting the revert). The PRE_ test runs some time before. From the logs:

../../chrome/browser/browsing_data/browsing_data_remover_browsertest.cc:943: Failure
Expected equality of these values:
  1
  GetMediaLicenseCount()
    Which is: 0
../../chrome/browser/browsing_data/browsing_data_remover_browsertest.cc:944: Failure
Value of: HasDataForType(kMediaLicenseType)
  Actual: false
Expected: true

The failures (on lines 943 & 944) happen after the test has created the second license, and deleted what should have only been the first based on the time (it's testing that the newer license is still around). However, it looks like both media licenses have been deleted. The deletion time is based on the start time of the test, before it creates the second license. I added logging locally, and here are the times shown:
  Test starting @ 2018-11-17 02:06:57.243 UTC
  Found 1 licenses.
  2018-11-17 02:06:48.820 UTC (created by the PRE_ test)
  // create second license
  Found 2 licenses.
  2018-11-17 02:06:48.820 UTC (original)
  2018-11-17 02:06:57.911 UTC (new license)
  // delete up to 02:06:57.243
  Found 1 licenses.
  2018-11-17 02:06:57.911 UTC (only new left)
  // delete everything
  Found 0 licenses.
The second license seems sufficiently newer (0.668s), but maybe it somehow matches <= start_time. Maybe the browser uses a different time than the test (as file time is based on what the browser thinks), or the time granularity is too small (test runs too fast?, but we load the web page after getting the start time).
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 20

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

commit 45a0df8f9f8fd9e4e8a1d4fbee883bca96639eb7
Author: John Rummell <jrummell@chromium.org>
Date: Tue Nov 20 18:28:31 2018

Reland "Update BrowsingDataRemoverBrowserTest to include MediaLicenses"

Original change's description:
> MediaLicenses can be cleared from the ClearBrowsingData dialog, so update the
> BrowsingDataRemoverBrowserTest to check them as well. This uses the test-only
> External Clear Key CDM to store the license in the file system, if it is
> available.
>

This reverts commit 72f2d4cd06bcaaad1a8f91a9b7b5559ecf70b8d9.

BUG=808690, 879812 
TEST=new browser_tests pass

Change-Id: Ie9e9229fc85244eed922d93dce82cb3fa09bbe5c
Reviewed-on: https://chromium-review.googlesource.com/c/1342779
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609752}
[modify] https://crrev.com/45a0df8f9f8fd9e4e8a1d4fbee883bca96639eb7/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[add] https://crrev.com/45a0df8f9f8fd9e4e8a1d4fbee883bca96639eb7/content/test/data/browsing_data/media_license.html

Cc: dullweber@chromium.org
Passed CQ, but I see some failures. It appears the file system on Mac rounds time to nearest second:
    MediaLicenseTimedDeletion starting @ 2018-11-20 20:01:32.545 UTC
    Found 1 licenses.
    2018-11-20 20:00:25.000 UTC
    Found 2 licenses.
    2018-11-20 20:00:25.000 UTC
    2018-11-20 20:01:32.000 UTC    <-- new file has time before test start time.
    Found 0 licenses.
    Failure
    Expected equality of these values:
      1
      GetMediaLicenseCount()
        Which is: 0

I'll revert my change, and see if I can find a solution that handles this.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 20

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

commit e9fc38c7b46443cb953b7676fe0f4e82c3d9c0dc
Author: John Rummell <jrummell@chromium.org>
Date: Tue Nov 20 20:49:47 2018

Revert "Reland "Update BrowsingDataRemoverBrowserTest to include MediaLicenses""

This reverts commit 45a0df8f9f8fd9e4e8a1d4fbee883bca96639eb7.

Reason for revert: Mac tests flaky, due to filesystem storing time in second increments only.

Original change's description:
> Reland "Update BrowsingDataRemoverBrowserTest to include MediaLicenses"
> 
> Original change's description:
> > MediaLicenses can be cleared from the ClearBrowsingData dialog, so update the
> > BrowsingDataRemoverBrowserTest to check them as well. This uses the test-only
> > External Clear Key CDM to store the license in the file system, if it is
> > available.
> >
> 
> This reverts commit 72f2d4cd06bcaaad1a8f91a9b7b5559ecf70b8d9.
> 
> BUG=808690, 879812 
> TEST=new browser_tests pass
> 
> Change-Id: Ie9e9229fc85244eed922d93dce82cb3fa09bbe5c
> Reviewed-on: https://chromium-review.googlesource.com/c/1342779
> Reviewed-by: Christian Dullweber <dullweber@chromium.org>
> Commit-Queue: John Rummell <jrummell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#609752}

TBR=jrummell@chromium.org,dullweber@chromium.org

Change-Id: I1c49014959d10fb9eb2306af0f71cb724ef40070
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 808690,  879812 
Reviewed-on: https://chromium-review.googlesource.com/c/1344884
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609790}
[modify] https://crrev.com/e9fc38c7b46443cb953b7676fe0f4e82c3d9c0dc/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[delete] https://crrev.com/622c02f9a7e9fe8d55bab94de43b3dab30b8b9b8/content/test/data/browsing_data/media_license.html

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 21

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

commit a4831e9b528dba6992024d0f2242f3b1bad07f9a
Author: John Rummell <jrummell@chromium.org>
Date: Wed Nov 21 21:10:00 2018

Reland "Update BrowsingDataRemoverBrowserTest to include MediaLicenses"

Original change's description:
> MediaLicenses can be cleared from the ClearBrowsingData dialog, so update the
> BrowsingDataRemoverBrowserTest to check them as well. This uses the test-only
> External Clear Key CDM to store the license in the file system, if it is
> available.

This reverts commit e9fc38c7b46443cb953b7676fe0f4e82c3d9c0dc.

The original CL was flaky due to Mac's only saving file timestamps to
second granularity. As a result it was possible for the newly created license
to be saved with a timestamp prior to the current actual time, and deleting
"old" licenses would include it in the deletion. Change is to wait for some
time on Macs only to ensure that the "new" license has a later timestamp.

BUG=808690, 879812 
TEST=new browser_tests pass

Change-Id: Ibd7cf65b468f98af9aa583d3f320e54c8f3f223e
Reviewed-on: https://chromium-review.googlesource.com/c/1345248
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610211}
[modify] https://crrev.com/a4831e9b528dba6992024d0f2242f3b1bad07f9a/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[add] https://crrev.com/a4831e9b528dba6992024d0f2242f3b1bad07f9a/content/test/data/browsing_data/media_license.html

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 22

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

commit b6bfa2b7f9065773ba9df3b492e4cc5559e4e785
Author: Christian Dullweber <dullweber@chromium.org>
Date: Thu Nov 22 10:42:47 2018

Revert "Reland "Update BrowsingDataRemoverBrowserTest to include MediaLicenses""

This reverts commit a4831e9b528dba6992024d0f2242f3b1bad07f9a.

Reason for revert: still flaky :(

Original change's description:
> Reland "Update BrowsingDataRemoverBrowserTest to include MediaLicenses"
> 
> Original change's description:
> > MediaLicenses can be cleared from the ClearBrowsingData dialog, so update the
> > BrowsingDataRemoverBrowserTest to check them as well. This uses the test-only
> > External Clear Key CDM to store the license in the file system, if it is
> > available.
> 
> This reverts commit e9fc38c7b46443cb953b7676fe0f4e82c3d9c0dc.
> 
> The original CL was flaky due to Mac's only saving file timestamps to
> second granularity. As a result it was possible for the newly created license
> to be saved with a timestamp prior to the current actual time, and deleting
> "old" licenses would include it in the deletion. Change is to wait for some
> time on Macs only to ensure that the "new" license has a later timestamp.
> 
> BUG=808690, 879812 
> TEST=new browser_tests pass
> 
> Change-Id: Ibd7cf65b468f98af9aa583d3f320e54c8f3f223e
> Reviewed-on: https://chromium-review.googlesource.com/c/1345248
> Reviewed-by: Christian Dullweber <dullweber@chromium.org>
> Commit-Queue: John Rummell <jrummell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#610211}

TBR=jrummell@chromium.org,dullweber@chromium.org

Change-Id: I31ee32ae7bb490ce799c92244f352e5a9a627552
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 808690,  879812 ,  907799 
Reviewed-on: https://chromium-review.googlesource.com/c/1347367
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610369}
[modify] https://crrev.com/b6bfa2b7f9065773ba9df3b492e4cc5559e4e785/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc
[delete] https://crrev.com/ea9f7c1f73dbe189791ad403b351540620d4ef8b/content/test/data/browsing_data/media_license.html

Status: Fixed (was: Assigned)
Code has been reverted, so marking this as fixed as the test is no longer running. Original issue 808690 will continue tracking attempts to get the test working again.

Sign in to add a comment