Bug 226124 – Make error returned by SQLiteIDBBackingStore::getOrEstablishDatabaseInfo more detailed

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Sihui Liu]

To help debug rdar://77532291.

[Sihui Liu]

Created attachment 429361 [details]
Patch

[Sihui Liu]

Created attachment 429562 [details]
Patch

[Chris Dumez]

Comment on attachment 429562 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:-262
> -            LOG_ERROR("Unable to prepare statement to fetch schema for the Records table.");

Unclear why this patch is getting rid of all the LOG_ERROR(). You can both LOG_ERROR AND return a useful IDBError. Why trade one for the other?

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:257
> +    String tableStatement = database.tableSQL("Records"_s);

Why is it named Statement and not schema like it used to be? If I see statement, I expect a SQLiteStatement.

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:262
> +        return IDBError { };

wouldn't `return { };` work?

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:267
> +        return IDBError { };

wouldn't `return { };` work?

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:293
> +    return IDBError { };

wouldn't `return { };` work?

[Sihui Liu]

(In reply to Chris Dumez from comment #3)
> Comment on attachment 429562 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=429562&action=review
> 
> > Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:-262
> > -            LOG_ERROR("Unable to prepare statement to fetch schema for the Records table.");
> 
> Unclear why this patch is getting rid of all the LOG_ERROR(). You can both
> LOG_ERROR AND return a useful IDBError. Why trade one for the other?
> 

As the error is more detailed, we can add error logging at higher level.
Will add logging in UniqueIDBDatabase::performCurrentOpenOperation().

> > Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:257
> > +    String tableStatement = database.tableSQL("Records"_s);
> 
> Why is it named Statement and not schema like it used to be? If I see
> statement, I expect a SQLiteStatement.
> 

I think schema is for describing the whole database and this is a column of an entry in schema table, which is usually text of the original statement to create the table.

> > Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:262
> > +        return IDBError { };
> 
> wouldn't `return { };` work?
> 

No, chosen constructor is explicit in copy-initialization. (IDBError's constructor is explicit, we may change that)

> > Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:267
> > +        return IDBError { };
> 
> wouldn't `return { };` work?
> 
> > Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:293
> > +    return IDBError { };
> 
> wouldn't `return { };` work?

[Sihui Liu]

Created attachment 429695 [details]
Patch for landing

[EWS]

Committed r278053 (238138@main): <https://commits.webkit.org/238138@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 429695 [details].

[Radar WebKit Bug Importer]

<rdar://problem/78480040>