| Commit-Queue | +1 |
migration_paths_ = std::move(result.migration_paths);Xiaohan ZhaoInstead of creating these path in DomStorageDatabase, we could just forward the storage_type and storage_partition_dir to this cb from Open() and build the paths here if (!is_sqlite_).
Done
base::FilePath leveldb_path =
DomStorageDatabase::GetLevelDbPath(storage_type, storage_partition_dir);
base::FilePath sqlite_path =
DomStorageDatabase::GetSqlitePath(storage_type, storage_partition_dir);
OpenResult::MigrationPaths migration_paths{
.leveldb_path = leveldb_path,
.sqlite_path = sqlite_path,
.sqlite_staging_path = sqlite_path.AddExtensionASCII("migrating"),Xiaohan ZhaoThese don't need to be populated here, do they? We have access to these helper functions in AsyncDomStorageDatabase. I think the only thing we are integrating into the Open and initialize flow for is to CheckLevelDBStatus. So maybe all you really need is a `bool is_level_db_present`. And even that we can deduce from OpenResult::is_sqlite, no?
Because `bool is_level_db_present = !is_sqlite;`
So can we forego any changes to OpenResult and in the code leading up to InitializeDatabase?
Done
{DomStorageSqliteRolloutStage::kMigrateLevelDbToSqlite,
"MigrateLevelDbToSqlite"},Xiaohan ZhaoIt feels odd to have this be a param for the NewDatabases feature. Migration will move *existing* databases from LevelDB to SQLite based backend. Should this be its own feature like `kDomStorageSqliteMigration` or do we need this approach for some reason. If so, should we add a comment to that effect?
Done. Added the new feature `kDomStorageSqliteMigration` and removed this `DomStorageSqliteRolloutStage::kMigrateLevelDbToSqlite`. Thanks!
// Migration uses a dedicated open path and never consults this helper.Xiaohan ZhaoShould this be a NOTREACHED then?
Done
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |