[webkit-reviews] review granted: [Bug 201002] Try to recover nicely when getting an unexpected schema in the service workers database : [Attachment 376941] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 201002: Try to recover nicely when getting an unexpected schema in the
service workers database
https://bugs.webkit.org/show_bug.cgi?id=201002

Attachment 376941: Patch

https://bugs.webkit.org/attachment.cgi?id=376941&action=review




--- Comment #4 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 376941
  --> https://bugs.webkit.org/attachment.cgi?id=376941
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376941&action=review

> Source/WebCore/workers/service/server/RegistrationDatabase.cpp:173
> +	   openSQLiteDatabase(fullFilename);

It seems a bit strange to be able to recover ensureValidRecordsTable but not
importRecords error.
An error in importRecords is probably unrecoverable as well. In that case, we
currently proceed with zero registration but we will no longer be able to push
changes since push changes try to import before pushing changes.

m_database->open error cases seem more difficult.
Some error cases are recoverable (process suspension) but others are probably
not recoverable.
It might be safer to not flush it for now.

We could cover all these cases by calling
SQLiteFileSystem::deleteDatabaseFile(fullFilename) in scopeExit conditionally
on a boolean storing whether recoverable or not.

As a side note, it seems that if process suspension happens exactly at the time
we try to open the database to import the service worker registrations, we will
load zero registrations.
Then, when being unsuspended, we will consider that there are zero
registrations. At the first push change, we will import the registrations
before pushing the changes.
I am not sure this code path is correct. Maybe we should make sure to restart
import at unsuspension time.