Bug 104422 - IndexedDB: Propagate more leveldb errors to script
Summary: IndexedDB: Propagate more leveldb errors to script
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Grogan
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-07 18:06 PST by David Grogan
Modified: 2012-12-10 16:22 PST (History)
4 users (show)

See Also:


Attachments
Patch (23.09 KB, patch)
2012-12-07 18:07 PST, David Grogan
no flags Details | Formatted Diff | Diff
Patch (23.05 KB, patch)
2012-12-07 18:11 PST, David Grogan
no flags Details | Formatted Diff | Diff
Patch (24.36 KB, patch)
2012-12-07 18:16 PST, David Grogan
no flags Details | Formatted Diff | Diff
Patch (22.19 KB, patch)
2012-12-07 18:22 PST, David Grogan
no flags Details | Formatted Diff | Diff
Patch (22.25 KB, patch)
2012-12-07 18:37 PST, David Grogan
no flags Details | Formatted Diff | Diff
Patch (22.69 KB, patch)
2012-12-10 14:55 PST, David Grogan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Grogan 2012-12-07 18:06:49 PST
IndexedDB: Propagate all leveldb errors to script
Comment 1 David Grogan 2012-12-07 18:07:02 PST
Created attachment 178314 [details]
Patch
Comment 2 David Grogan 2012-12-07 18:11:07 PST
Created attachment 178316 [details]
Patch
Comment 3 David Grogan 2012-12-07 18:16:09 PST
Created attachment 178318 [details]
Patch
Comment 4 David Grogan 2012-12-07 18:22:45 PST
Created attachment 178319 [details]
Patch
Comment 5 WebKit Review Bot 2012-12-07 18:29:19 PST
Attachment 178319 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 David Grogan 2012-12-07 18:37:55 PST
Created attachment 178321 [details]
Patch
Comment 7 David Grogan 2012-12-07 18:43:26 PST
Josh or Alec, could you take a look?

Everything remaining is a consistency error, rather than a leveldb IO error.
Comment 8 Joshua Bell 2012-12-10 11:52:13 PST
Comment on attachment 178321 [details]
Patch

Overall LGTM (s/good/awesome/)

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

> Source/WebCore/ChangeLog:11
> +        Nothing called getBool, so it is deleted.

Yeah, I had a FIXME to add it but I don't think anything was converted over to use it. :P

> Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:378
> +                InternalError(IDBLevelDBBackingStoreReadErrorSetupMetadata);

Other places in open() eschew InternalError and increment a specific histogram (since we'd hope to catch most errors here). Do the same thing here, or stick with InternalError?

> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:471
> +        transaction->abort(error);

Use error.release()
Comment 9 David Grogan 2012-12-10 14:39:25 PST
Comment on attachment 178321 [details]
Patch

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

>> Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp:378
>> +                InternalError(IDBLevelDBBackingStoreReadErrorSetupMetadata);
> 
> Other places in open() eschew InternalError and increment a specific histogram (since we'd hope to catch most errors here). Do the same thing here, or stick with InternalError?

Changed.

>> Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:471
>> +        transaction->abort(error);
> 
> Use error.release()

Done.
Comment 10 David Grogan 2012-12-10 14:55:25 PST
Created attachment 178638 [details]
Patch
Comment 11 David Grogan 2012-12-10 15:45:45 PST
Tony, could you review this?
Comment 12 WebKit Review Bot 2012-12-10 16:22:14 PST
Comment on attachment 178638 [details]
Patch

Clearing flags on attachment: 178638

Committed r137223: <http://trac.webkit.org/changeset/137223>
Comment 13 WebKit Review Bot 2012-12-10 16:22:18 PST
All reviewed patches have been landed.  Closing bug.