Bug 156713 – [Win][IndexedDB] Fix build errors.

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

There are currently a few compile and link errors when building WinCairo with IndexedDB enabled.

Created attachment 276664 [details]
Patch

Created attachment 276667 [details]
Patch

[WebKit Commit Bot]

Attachment 276667 [details] did not pass style-queue:


ERROR: Source/WebCore/PlatformWin.cmake:189:  There should be exactly one empty line instead of 0 between "Modules/indexeddb" and "Modules/indexeddb/client".  [list/emptyline] [5]
ERROR: Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:1188:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

Comment on attachment 276667 [details]
Patch

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

cool!

> Source/WebCore/Modules/indexeddb/server/MemoryBackingStoreTransaction.cpp:44
> -    return std::make_unique<MemoryBackingStoreTransaction>(backingStore, info);
> +    return std::unique_ptr<MemoryBackingStoreTransaction>(new MemoryBackingStoreTransaction(backingStore, info));

This shouldn't be needed.

> Source/WebCore/Modules/indexeddb/server/MemoryIDBBackingStore.cpp:48
> -    return std::make_unique<MemoryIDBBackingStore>(identifier);
> +    return std::unique_ptr<MemoryIDBBackingStore>(new MemoryIDBBackingStore(identifier));

This shouldn't be needed.

> Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:1188
> -    auto callback = [this, self, refTransaction](const IDBError& error) {
> +    ErrorCallback callback = [this, self, refTransaction](const IDBError& error) {

This shouldn't be needed.

> Source/WebKit/win/storage/WebDatabaseProvider.cpp:45
> +    sprintf_s(databaseDirectory, MAX_PATH, "%s\\%s", appDataDirectory, executableName);

On Mac we use .../WebKit/Databases/___IndexedDB.  We should probably do something similar here.

[Alex Christensen]

Comment on attachment 276667 [details]
Patch

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

> Source/WebKit/win/WebView.cpp:5062
> +    RuntimeEnabledFeatures::sharedFeatures().setWebkitIndexedDBEnabled(true);

This should probably be false by default, then set to true for DumpRenderTree and MiniBrowser.

(In reply to comment #4)
> Comment on attachment 276667 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276667&action=review
> 
> cool!
> 

Thanks for reviewing :)

> > Source/WebCore/Modules/indexeddb/server/MemoryBackingStoreTransaction.cpp:44
> > -    return std::make_unique<MemoryBackingStoreTransaction>(backingStore, info);
> > +    return std::unique_ptr<MemoryBackingStoreTransaction>(new MemoryBackingStoreTransaction(backingStore, info));
> 
> This shouldn't be needed.
> 

I get a compile error without this:

error C2248: 'WebCore::IDBServer::MemoryBackingStoreTransaction::MemoryBackingStoreTransaction': cannot access private member declared in class 'WebCore::IDBServer::MemoryBackingStoreTransaction

Maybe we should fix this in another way?

> 
> > Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp:1188
> > -    auto callback = [this, self, refTransaction](const IDBError& error) {
> > +    ErrorCallback callback = [this, self, refTransaction](const IDBError& error) {
> 
> This shouldn't be needed.
> 

I get an error without this:

error C2668: 'WebCore::IDBServer::UniqueIDBDatabase::storeCallback': ambiguous call to overloaded function

> > Source/WebKit/win/storage/WebDatabaseProvider.cpp:45
> > +    sprintf_s(databaseDirectory, MAX_PATH, "%s\\%s", appDataDirectory, executableName);
> 
> On Mac we use .../WebKit/Databases/___IndexedDB.  We should probably do
> something similar here.

I will update the path :)

Created attachment 276755 [details]
Patch

[WebKit Commit Bot]

Attachment 276755 [details] did not pass style-queue:


ERROR: Source/WebCore/PlatformWin.cmake:189:  There should be exactly one empty line instead of 0 between "Modules/indexeddb" and "Modules/indexeddb/client".  [list/emptyline] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

(In reply to comment #4)
> Comment on attachment 276667 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276667&action=review
> 
> cool!
> 
> > Source/WebCore/Modules/indexeddb/server/MemoryBackingStoreTransaction.cpp:44
> > -    return std::make_unique<MemoryBackingStoreTransaction>(backingStore, info);
> > +    return std::unique_ptr<MemoryBackingStoreTransaction>(new MemoryBackingStoreTransaction(backingStore, info));
> 
> This shouldn't be needed.
> 

I am afraid I didn't find a better way to fix these compile errors.
The main problem seems to be that MSVC does not accept that std::make_unique is declared as a friend of a class.

[Alex Christensen]

Comment on attachment 276755 [details]
Patch

Just make the constructor public.

Created attachment 276760 [details]
Patch

(In reply to comment #10)
> Comment on attachment 276755 [details]
> Patch
> 
> Just make the constructor public.

Thanks :) Updated patch.

[WebKit Commit Bot]

Attachment 276760 [details] did not pass style-queue:


ERROR: Source/WebCore/PlatformWin.cmake:189:  There should be exactly one empty line instead of 0 between "Modules/indexeddb" and "Modules/indexeddb/client".  [list/emptyline] [5]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

Comment on attachment 276760 [details]
Patch

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

Looks good.  I think this needs another iteration, though.

> Source/WebCore/PlatformWin.cmake:189
>      Modules/indexeddb
> +    Modules/indexeddb/client

Style bot wants a space here.

> Source/WebCore/bindings/js/JSDOMWindowBase.h:23
> +#include "DOMWindow.h"

Is this really necessary?  What error does this prevent?  Can we not forward declare things?  Including more headers in a header like this makes the build slower.

> Source/WebCore/bindings/js/JSDOMWrapper.h:25
> +#include "DOMWrapperWorld.h"

Ditto.

> Source/WebKit/win/storage/WebDatabaseProvider.cpp:41
> +        return String("");

This isn't used in WebKit.  I think this is what emptyString() is for.

> Source/WebKit/win/storage/WebDatabaseProvider.cpp:50
> +        return String("");

ditto.

Created attachment 276924 [details]
Patch

(In reply to comment #14)
> Comment on attachment 276760 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=276760&action=review
> 

Thanks for reviewing!

> 
> > Source/WebCore/bindings/js/JSDOMWindowBase.h:23
> > +#include "DOMWindow.h"
> 
> Is this really necessary?  What error does this prevent?  Can we not forward
> declare things?  Including more headers in a header like this makes the
> build slower.
> 

I get the error:

wtf/PassRefPtr.h(42): error C2027: use of undefined type 'WebCore::DOMWindow'

PassRefPtr<DOMWindow> is used in JSDOMWindowBase.h, but this fails to compile since the class DOMWindow is only forward declared here.

[WebKit Commit Bot]

Comment on attachment 276924 [details]
Patch

Clearing flags on attachment: 276924

Committed r200036: <http://trac.webkit.org/changeset/200036>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.