Bug 124819 – DatabaseProcess: Add "UniqueIDBDatabase" that multiple WebProcesses can connect to

• Component: WebKit2
• Status: RESOLVED FIXED

[Brady Eidson]

DatbaseProcess: Add "UniquedIDBDatabase" that multiple WebProcesses can connect to

[Brady Eidson]

Created attachment 217764 [details]
Patch v1

[EFL EWS Bot]

Comment on attachment 217764 [details]
Patch v1

Attachment 217764 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/35328266

[Brady Eidson]

Created attachment 217765 [details]
Patch v2 - Fix includes leftover from a stash merge.

Comment on attachment 217765 [details]
Patch v2 - Fix includes leftover from a stash merge.

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

> Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:81
> +    RefPtr<UniqueIDBDatabase> database = m_idbDatabases.get(identifier);
> +    if (database)
> +        return database;
> +
> +    database = UniqueIDBDatabase::create(identifier);
> +    m_idbDatabases.set(identifier, database);
> +
> +    return database.release();

Is there a way to do this without hashing twice in the not-found case? I think there is (using HashMap::find).

> Source/WebKit2/DatabaseProcess/IndexedDB/DatabaseProcessIDBConnection.h:70
> +    UniqueIDBDatabaseIdentifier m_databaseIdentifier;

Why do we need this as a member variable here and can’t rely on getting the identifier from the m_uniqueIDBDatabase member?

> Source/WebKit2/DatabaseProcess/IndexedDB/UniqueIDBDatabaseIdentifier.h:58
> +    mutable String m_identifierString;

This doesn’t appear to be used.

[Brady Eidson]

http://trac.webkit.org/changeset/159737

[Darin Adler]

Comment on attachment 217765 [details]
Patch v2 - Fix includes leftover from a stash merge.

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

>> Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp:81
>> +    RefPtr<UniqueIDBDatabase> database = m_idbDatabases.get(identifier);
>> +    if (database)
>> +        return database;
>> +
>> +    database = UniqueIDBDatabase::create(identifier);
>> +    m_idbDatabases.set(identifier, database);
>> +
>> +    return database.release();
> 
> Is there a way to do this without hashing twice in the not-found case? I think there is (using HashMap::find).

Here’s a good way to write it without hashing twice:

    RefPtr<UniqueIDDatabase>& database = m_idbDatabases.add(identifier, nullptr).iterator->value;
    if (!database)
        database = UniqueIDBDatabase::create(identifier);
    return database;

We use this idiom a lot.

> Source/WebKit2/DatabaseProcess/IndexedDB/DatabaseProcessIDBConnection.cpp:66
> +    m_databaseIdentifier = std::move(UniqueIDBDatabaseIdentifier(databaseName, openingOrigin, mainFrameOrigin));

I don’t think this std::move is needed or helpful.