New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 698819 link

Starred by 1 user
Status: Fixed
Owner:
█ holte@chromium.org
Last visit > 30 days ago
Closed: Mar 2017
Cc:
skare@chromium.org
isherman@chromium.org
asvitk...@chromium.org
█ rkaplow@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

UKM service logic around persisted_logs_.DiscardStagedLog() is not correct

Project Member Reported by asvitk...@chromium.org, Mar 6 2017 
UKM service logic around persisted_logs_.DiscardStagedLog() is not correct.

 crbug.com/695433 shows crashes when DiscardStagedLog() was called when there was no staged log.

I'm adding a workaround for the crash under crbug.com/695433, but we should have a more robust longer term fix for this, which this bug is meant to track.

Comment 1 by holte@chromium.org, Mar 6 2017 

Looking at the code again, I think a way that this could happen is if we call UkmService::Purge after StartScheduledUpload has been run.  This would happen if the user clears history, or opts out of sync/uma during that time.  I think Purge and OnUploadComplete should be the only places we discard staged logs from.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 6 2017 

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

commit c88119049586f4a9e28fd8c2ba1fa7ccc25c335f
Author: asvitkine <asvitkine@chromium.org>
Date: Mon Mar 06 23:36:59 2017

Fix UKM crash. Add TODO for fixing this properly.

BUG= 698819 ,695433

Review-Url: https://codereview.chromium.org/2736733004
Cr-Commit-Position: refs/heads/master@{#454998}

[modify] https://crrev.com/c88119049586f4a9e28fd8c2ba1fa7ccc25c335f/components/ukm/ukm_service.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 7 2017

Labels: merge-merged-3029
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cdd9e7590fdd5e39222d8e99515f1a02a6bc0c14

commit cdd9e7590fdd5e39222d8e99515f1a02a6bc0c14
Author: Alexei Svitkine <asvitkine@chromium.org>
Date: Tue Mar 07 15:32:04 2017

Fix UKM crash. Add TODO for fixing this properly.

BUG= 698819 ,695433

Review-Url: https://codereview.chromium.org/2736733004
Cr-Commit-Position: refs/heads/master@{#454998}
(cherry picked from commit c88119049586f4a9e28fd8c2ba1fa7ccc25c335f)

Review-Url: https://codereview.chromium.org/2735153003 .
Cr-Commit-Position: refs/branch-heads/3029@{#41}
Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471}

[modify] https://crrev.com/cdd9e7590fdd5e39222d8e99515f1a02a6bc0c14/components/ukm/ukm_service.cc

Comment 4 by holte@chromium.org, Mar 22 2017

Status: Fixed (was: Assigned)
This should be fixed by https://codereview.chromium.org/2752073002/
had the wrong bug# on the CL.

Comment 5 by holte@chromium.org, Mar 22 2017 

Actually it was https://codereview.chromium.org/2736683003, the other CL was a different issue.
►

Sign in to add a comment