New issue
Advanced search Search tips

Issue 829125 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 0
Type: Bug



Sign in to add a comment

IndexedDB databases version 2 delete all blobs on open.

Project Member Reported by dmu...@chromium.org, Apr 4 2018

Issue description

Because we fail to save the new schema version to IndexedDB (schema version 3), we always delete blobs every time we open an old IndexedDB database with schema version 2.

We need to stop deleting blobs to stop the bleeding - then we can do something about detecting corrupt databases and deleting them.
 
Labels: -Pri-3 OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows Pri-0
Labels: -OS-Fuchsia
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 5 2018

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

commit 3c0d175b5584a2e5be9d493aca8ddd46c7d78e12
Author: Daniel Murphy <dmurph@chromium.org>
Date: Thu Apr 05 04:35:38 2018

[IndexedDB] Stop deleting blobs on open for schema version 2 dbs

Bug:  756447 ,  829125 , 829141
Change-Id: I854594ed35fc5a99df1b68534db42bcb65b81a0f
Reviewed-on: https://chromium-review.googlesource.com/996582
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548325}
[modify] https://crrev.com/3c0d175b5584a2e5be9d493aca8ddd46c7d78e12/content/browser/indexed_db/indexed_db_backing_store.cc

Labels: Merge-Request-66
Requesting a merge, the will 'stop the bleeding' of old databases where we're mistakenly deleting blobs on every db open.
Project Member

Comment 5 by sheriffbot@chromium.org, Apr 5 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: We are only 11 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 6 by cmasso@google.com, Apr 5 2018

Please verify in canary
How does change look in Canary? 
No crashes or spikes of UMA errors. I think it's safe to land.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 10 2018

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

commit 705d7330d828101a658d390e8ba0790b8a6d29be
Author: Daniel Murphy <dmurph@chromium.org>
Date: Tue Apr 10 02:53:28 2018

[IndexedDB] Schema upgrade fix, fail for corrupt blobs, fix enums.xml

For the last 4 years we 'saved' the new schema version number, so any v2
databases are never marked as v3. The 'migration' always deleted all
blob files. To correctly upgrade to v3, all v2 databases that have blob
entries are considered corrupt and are wiped.

This also fixes the enums.xml values for IDB setup errors, which have
been missing 'DeleteIndex' and 'ClearObjectStore' (and subsequently have
had incorrect reporting on values > 23) since 2014:
https://crrev.com/b05831ee

Bug:  756447 ,  829125 , 829141
Change-Id: I1d711b5ed4bfb4e7240da56a2fc78e6ebe561ca1
Reviewed-on: https://chromium-review.googlesource.com/1000138
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549395}
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_backing_store.cc
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_backing_store.h
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_backing_store_unittest.cc
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_leveldb_operations.h
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_metadata_coding.cc
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_metadata_coding.h
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/content/browser/indexed_db/indexed_db_reporting.h
[modify] https://crrev.com/705d7330d828101a658d390e8ba0790b8a6d29be/tools/metrics/histograms/enums.xml

Labels: -Merge-Review-66 Merge-Approved-66
Approving merge to M66. Branch:3359
Project Member

Comment 11 by bugdroid1@chromium.org, Apr 10 2018

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

commit 530acd0a64c89b4ae2f64f6199035db2c66aa5a0
Author: Alan Cutter <alancutter@chromium.org>
Date: Tue Apr 10 04:17:52 2018

Revert "[IndexedDB] Schema upgrade fix, fail for corrupt blobs, fix enums.xml"

This reverts commit 705d7330d828101a658d390e8ba0790b8a6d29be.

Reason for revert: Broke WinMSVC64 (dbg) compile.
https://ci.chromium.org/buildbot/chromium.win/WinMSVC64%20%28dbg%29/4682
indexed_db_backing_store_unittest.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) class leveldb::Status __cdecl content::indexed_db::GetInt<class content::LevelDBTransaction>(class content::LevelDBTransaction *,class base::BasicStringPiece<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > > const &,__int64 *,bool *)" (__imp_??$GetInt@VLevelDBTransaction@content@@@indexed_db@content@@YA?AVStatus@leveldb@@PEAVLevelDBTransaction@1@AEBV?$BasicStringPiece@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@base@@PEA_JPEA_N@Z) referenced in function "public: void __cdecl <lambda_395e582d16a7cd048cd3868c3a9ede8f>::operator()(class content::IndexedDBBackingStore *,class content::IndexedDBKey,struct content::IndexedDBValue,struct `private: virtual void __cdecl content::indexed_db_backing_store_unittest::IndexedDBBackingStoreTest_SchemaUpgradeWithoutBlobsSurvives_Test::TestBody(void)'::`2'::TestState *)const " (??R<lambda_395e582d16a7cd048cd3868c3a9ede8f>@@QEBAXPEAVIndexedDBBackingStore@content@@VIndexedDBKey@2@UIndexedDBValue@2@PEAUTestState@?1??TestBody@IndexedDBBackingStoreTest_SchemaUpgradeWithoutBlobsSurvives_Test@indexed_db_backing_store_unittest@2@EEAAXXZ@@Z)

Original change's description:
> [IndexedDB] Schema upgrade fix, fail for corrupt blobs, fix enums.xml
> 
> For the last 4 years we 'saved' the new schema version number, so any v2
> databases are never marked as v3. The 'migration' always deleted all
> blob files. To correctly upgrade to v3, all v2 databases that have blob
> entries are considered corrupt and are wiped.
> 
> This also fixes the enums.xml values for IDB setup errors, which have
> been missing 'DeleteIndex' and 'ClearObjectStore' (and subsequently have
> had incorrect reporting on values > 23) since 2014:
> https://crrev.com/b05831ee
> 
> Bug:  756447 ,  829125 , 829141
> Change-Id: I1d711b5ed4bfb4e7240da56a2fc78e6ebe561ca1
> Reviewed-on: https://chromium-review.googlesource.com/1000138
> Reviewed-by: Mark Pearson <mpearson@chromium.org>
> Reviewed-by: Joshua Bell <jsbell@chromium.org>
> Commit-Queue: Daniel Murphy <dmurph@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#549395}

TBR=mpearson@chromium.org,jsbell@chromium.org,dmurph@chromium.org,pwnall@chromium.org

Change-Id: I563e3f3b77f0ff72025d3d398a6e8f5e36a818bc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  756447 ,  829125 , 829141
Reviewed-on: https://chromium-review.googlesource.com/1004133
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549413}
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_backing_store.cc
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_backing_store.h
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_backing_store_unittest.cc
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_leveldb_operations.h
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_metadata_coding.cc
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_metadata_coding.h
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/content/browser/indexed_db/indexed_db_reporting.h
[modify] https://crrev.com/530acd0a64c89b4ae2f64f6199035db2c66aa5a0/tools/metrics/histograms/enums.xml

Note - that revert was on a different change, not the one being merged.
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 10 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0d1aacb9239aae9ab57b1035cf62ceb1889e8c54

commit 0d1aacb9239aae9ab57b1035cf62ceb1889e8c54
Author: Daniel Murphy <dmurph@chromium.org>
Date: Tue Apr 10 18:23:18 2018

[IndexedDB] Stop deleting blobs on open for schema version 2 dbs

Bug:  756447 ,  829125 , 829141
Change-Id: I854594ed35fc5a99df1b68534db42bcb65b81a0f
Reviewed-on: https://chromium-review.googlesource.com/996582
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#548325}(cherry picked from commit 3c0d175b5584a2e5be9d493aca8ddd46c7d78e12)
Reviewed-on: https://chromium-review.googlesource.com/1005694
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#655}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/0d1aacb9239aae9ab57b1035cf62ceb1889e8c54/content/browser/indexed_db/indexed_db_backing_store.cc

Status: Fixed (was: Assigned)
Project Member

Comment 16 by bugdroid1@chromium.org, Apr 11 2018

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

commit 9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768
Author: Daniel Murphy <dmurph@chromium.org>
Date: Wed Apr 11 18:08:57 2018

[IndexedDB] Log schema version

Bug:  830909 ,  756447 ,  829125 , 829141
Change-Id: Iaf467fbe9cf2c836bfcd761d558c3f7c0e7379a8
Reviewed-on: https://chromium-review.googlesource.com/1003281
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#549653}(cherry picked from commit c757238fa2717fa08178def1faf522ad3c876416)
Reviewed-on: https://chromium-review.googlesource.com/1007942
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#684}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768/content/browser/indexed_db/indexed_db_backing_store.cc
[modify] https://crrev.com/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768/content/browser/indexed_db/indexed_db_leveldb_coding.h
[modify] https://crrev.com/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768/content/browser/indexed_db/indexed_db_reporting.cc
[modify] https://crrev.com/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768/content/browser/indexed_db/indexed_db_reporting.h
[modify] https://crrev.com/9ab30dca6ad03d9dc70f4ca9868a38f1c9b97768/tools/metrics/histograms/histograms.xml

Sign in to add a comment