New issue
Advanced search Search tips

Issue 605249 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Need to add recovery to the offline pages SQL implementation

Reported by bburns@chromium.org, Apr 20 2016

Issue description

Currently we don't detect (or recover from) corrupted SQL Lite DBs.  This is ok, since it's not in use yet.  However, before we turn it on, we need to enable this.

See:
https://code.google.com/p/chromium/codesearch#chromium/src/components/history/core/browser/thumbnail_database.cc&sq=package:chromium&type=cs&l=428&rcl=1461151052

for an example.

 
Owner: fgor...@chromium.org

Comment 2 by dim...@chromium.org, Sep 27 2016

Labels: -Type-Feature OS-Android Type-Bug
Status: Assigned (was: Available)
In the process of metrics analysis of this problem we discovered that clients, which hit the store load problems fall into 2 categories with respect to LoadStatus enum (where possible values are: LOAD_SUCCEEDED, STORE_INIT_FAILED, STORE_LOAD_FAILED, DATA_PARSING_FAILED). The categories are:
* Clients that consistently fail, showing 2, 2, 2, 2 (STORE_LOAD_FAILED) with every load,
* Clients that fail the load sometimes: 1 (STORE_INIT_FAILED), 0 (LOAD_SUCCEEDED), 0, 1, 1, 1, 0, which sometimes (presumably on slower devices) takes the shape of 1, 1, 1, 1, 1, 1, ...

The former case can be solved by doing the appropriate store reset and starting over. (After all these clients never get to load the store properly, so they don't have access to appropriate pages).

The latter case is one that exhibits a race condition between initial connection to the store and first load from the store. If the latter attempt is started before the former is finished, we are guaranteed to fail with LoadStatus::STORE_INIT_FAILED and the user cannot access offline page metadata, even thou there is a perfectly good database waiting for them.

We will address both problems as part of this bug.
Labels: -Pri-3 Pri-2
Status: Started (was: Assigned)
Marking as start and taking this thing on. Bumping priority because of race condition and impact on users with slower devices.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 17 2016

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

commit 9c4ec432de012e9f6c99dfc55cd0535694a4e828
Author: fgorski <fgorski@chromium.org>
Date: Thu Nov 17 21:59:19 2016

[Offline pages] Resetting offline page metadata store to handle LOAD/INIT failures

* Adds a store reset to offline page model, in case the initial load
  fails with STORE_LOAD_FAILED status
* Adds Initialize method to the store to handle STORE_INIT_FAILED.
* Postpones OfflinePageModel actions until after initialization of
  the store succeeds/fails.
* Adds tests to cover successful reset and unsuccessful one.
* Updates tests that needed adjusting.
BUG= 605249 

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

[modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_metadata_store.h
[modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_metadata_store_impl_unittest.cc
[modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_metadata_store_sql.cc
[modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_metadata_store_sql.h
[modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_model_impl.cc
[modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_model_impl.h
[modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_model_impl_unittest.cc
[modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_test_store.cc
[modify] https://crrev.com/9c4ec432de012e9f6c99dfc55cd0535694a4e828/components/offline_pages/offline_page_test_store.h

The following patch sorted the problem on RequestQueueStore side:



[Offline pages] Adding InitializeStoreTask to handle RQ store initialization and resets

* Adds RequestQueueStore::Initialize
* Adds InitializeStoreTask with tests.
* Updates RequestQueue to call store Initialize (using task)
* Updating tests for all RequestQueue related tasks
  (adds initialization)
* Moving in-memory store to test_support target
* Updating in-memory store with init/reset test scenarios
* Fixes minor bugs
* Removes unnecessary methods

BUG= 605249 ,  651238 
R=dougarnett@chromium.org,petewil@chromium.org

Committed: https://crrev.com/f871f672d0c9a16ded3374d0ed8667295b13898e
Cr-Commit-Position: refs/heads/master@{#435333}

Status: Fixed (was: Started)

Sign in to add a comment