Bug 124562 – Add WebIDBServerConnection and DatabaseProcessIDBConnection stubs

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

[Brady Eidson]

Add WebIDBServerConnection and DatabaseProcessIDBConnection stubs

Also remove Web/DatabaseProcessDatabaseBackend stubs, as that is no longer the abstraction layer.

[Brady Eidson]

Created attachment 217274 [details]
patch v1

[Alexey Proskuryakov]

Comment on attachment 217274 [details]
patch v1

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

> Source/WebKit2/DatabaseProcess/DatabaseToWebProcessConnection.cpp:84
> +    RefPtr<DatabaseProcessIDBConnection> connection = DatabaseProcessIDBConnection::create(backendIdentifier);

Just an idle thought, is that actually something that needs to be refcountable?

> Source/WebKit2/Shared/Databases/IndexedDB/IDBUtilities.cpp:40
> +    stringBuilder.append(openingOrigin.databaseIdentifier());

Can we get rid of databaseIdentifier() in IndexedDB? A comment in SecurityOrigin.cpp implies that we should:

    // Historically, we've used the following (somewhat non-sensical) string
    // for the databaseIdentifier of local files. We used to compute this
    // string because of a bug in how we handled the scheme for file URLs.
    // Now that we've fixed that bug, we still need to produce this string
    // to avoid breaking existing persistent state.

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:42
> +    DEFINE_STATIC_LOCAL(uint64_t, identifier, (1));

ASSERT(isMainThread());

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:43
> +    return identifier++;

Why not start with a 0, and return ++identifier?

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:55
> +

Empty line.

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:66
> +

Ditto.

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:71
> +

Ditto.

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:81
> +

Ditto.

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.cpp:86
> +

Looks like there are a few more...

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.h:31
> +#if ENABLE(INDEXED_DATABASE)
> +#if ENABLE(DATABASE_PROCESS)

Some of the new files have ENABLE(INDEXED_DATABASE) && ENABLE(DATABASE_PROCESS), for what it's worth.

> Source/WebKit2/WebProcess/Databases/IndexedDB/WebIDBServerConnection.h:85
> +    explicit WebIDBServerConnection(const String& databaseName, const WebCore::SecurityOrigin& openingOrigin, const WebCore::SecurityOrigin& mainFrameOrigin);

Is "explicit" of any use here?

[Brady Eidson]

(In reply to comment #2)

Most of these changed without comment.

> (From update of attachment 217274 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=217274&action=review
> 
> > Source/WebKit2/DatabaseProcess/DatabaseToWebProcessConnection.cpp:84
> > +    RefPtr<DatabaseProcessIDBConnection> connection = DatabaseProcessIDBConnection::create(backendIdentifier);
> 
> Just an idle thought, is that actually something that needs to be ref countable?

I think it will shortly.

> > Source/WebKit2/Shared/Databases/IndexedDB/IDBUtilities.cpp:40
> > +    stringBuilder.append(openingOrigin.databaseIdentifier());
> 
> Can we get rid of databaseIdentifier() in IndexedDB? A comment in SecurityOrigin.cpp implies that we should:
> 
>     // Historically, we've used the following (somewhat non-sensical) string
>     // for the databaseIdentifier of local files. We used to compute this
>     // string because of a bug in how we handled the scheme for file URLs.
>     // Now that we've fixed that bug, we still need to produce this string
>     // to avoid breaking existing persistent state.

Chatted with Alexey about this in person, came up with a slight alteration to the patch that will be landed soon.

[Brady Eidson]

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