Bug 225855 – Avoid more String creations when preparing SQLite statements

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

[Chris Dumez]

Avoid more String creations when preparing SQLite statements by using ASCIILiteral. Also rename the SQLiteDatabase::prepareString(const String&) overload so that it doesn't get called by mistake.

[Chris Dumez]

Created attachment 428799 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See https://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Chris Dumez]

Created attachment 428800 [details]
Patch

[Chris Dumez]

Created attachment 428801 [details]
Patch

[Chris Dumez]

Created attachment 428802 [details]
Patch

[Alex Christensen]

Comment on attachment 428802 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:136
> +    return "CREATE TABLE Records (objectStoreID INTEGER NOT NULL ON CONFLICT FAIL, key TEXT COLLATE IDBKEY NOT NULL ON CONFLICT FAIL, value NOT NULL ON CONFLICT FAIL, recordID INTEGER PRIMARY KEY)"_s;

We could use the preprocessor to put the common parts of this in one place.

> Source/WebCore/workers/service/server/RegistrationDatabase.cpp:73
>  static const String recordsTableSchemaAlternate()

It looks like this can be an ASCIILiteral too.

> Source/WebCore/workers/service/server/RegistrationDatabase.cpp:76
> +        "key TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE"

ditto on the duplicate code.  preprocessor

[Sihui Liu]

Comment on attachment 428802 [details]
Patch

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

> Source/WebKit/ChangeLog:10
> +        Avoid more String creations when preparing SQLite statements by using ASCIILiteral. Also rename the
> +        SQLiteDatabase::prepareStatement() / SQLiteDatabase::executeCommand() overloads that take in a 
> +        String to make sure they are not called by mistake.

In Source/WebCore/ChangeLog?

> Source/WebKitLegacy/ChangeLog:10
> +        Avoid more String creations when preparing SQLite statements by using ASCIILiteral. Also rename the
> +        SQLiteDatabase::prepareStatement() / SQLiteDatabase::executeCommand() overloads that take in a 
> +        String to make sure they are not called by mistake.

Ditto.

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:290
> +    if (!database.executeCommandSlow(v3RecordsTableSchema("_Temp_Records"))) {

static const String v3RecordsTableSchema(const String& tableName) is only used here with this change. Maybe just change it to static ASCIILiteral v3RecordsTableSchemaTemp()?

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:455
> +    if (!m_sqliteDB->executeCommandSlow(v3IndexRecordsTableSchema("_Temp_IndexRecords"))) {

Same as above. Make it static ASCIILiteral v3IndexRecordsTableSchemaTemp()?

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:596
> +    if (!m_sqliteDB->executeCommandSlow(indexInfoTableSchema("IndexInfo"_s))) {

Maybe convert static String indexInfoTableSchema(ASCIILiteral tableName) to static ASCIILiteral indexInfoTableSchema() and static ASCIILiteral indexInfoTableSchemaTemp()?

> Source/WebCore/Modules/indexeddb/server/SQLiteIDBBackingStore.cpp:783
> +    if (!database.executeCommandSlow(v3IndexRecordsTableSchema("_Temp_IndexRecords"))) {

static ASCIILiteral v3IndexRecordsTableSchemaTemp()?

[Chris Dumez]

Created attachment 428848 [details]
Patch

[EWS]

Committed r277601 (237818@main): <https://commits.webkit.org/237818@main>

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

[Radar WebKit Bug Importer]

<rdar://problem/78116715>

[Alex Christensen]

r277617

[Fujii Hironori]

Committed r277629 (237840@main): <https://commits.webkit.org/237840@main>