Make SERVICE_WORKER feature buildable on GTK, WPE

[Zan Dobersek]

Make SERVICE_WORKER feature buildable on GTK, WPE

[Zan Dobersek]

Created attachment 324382 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 324382 [details]
Patch

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

> Source/WebKit/PlatformGTK.cmake:1306
> -    COMMAND ${PERL_EXECUTABLE} ${WEBKIT_DIR}/Scripts/generate-forwarding-headers.pl --include-path ${WEBKIT_DIR} --output ${FORWARDING_HEADERS_DIR} --platform gtk --platform soup
> +    COMMAND ${PERL_EXECUTABLE} ${WEBKIT_DIR}/Scripts/generate-forwarding-headers.pl --include-path ${WEBKIT_DIR} --include-path ${DERIVED_SOURCES_WEBKIT_DIR} --output ${FORWARDING_HEADERS_DIR} --platform gtk --platform soup

I don't think this works in a clean build, the derived sources might not have been generated yet. This is a known issue that we normally solve by explicitly including the required headers in the non-generated source file that includes the generated one.

[Zan Dobersek]

Comment on attachment 324382 [details]
Patch

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

>> Source/WebKit/PlatformGTK.cmake:1306
>> +    COMMAND ${PERL_EXECUTABLE} ${WEBKIT_DIR}/Scripts/generate-forwarding-headers.pl --include-path ${WEBKIT_DIR} --include-path ${DERIVED_SOURCES_WEBKIT_DIR} --output ${FORWARDING_HEADERS_DIR} --platform gtk --platform soup
> 
> I don't think this works in a clean build, the derived sources might not have been generated yet. This is a known issue that we normally solve by explicitly including the required headers in the non-generated source file that includes the generated one.

Can this target not depend on the WebKit_DERIVED_SOURCES list, which would be the list of all source files generated via the generate-message-*.py scripts? Is there an existing bug covering this?

[Carlos Garcia Campos]

(In reply to Zan Dobersek from comment #3)
> Comment on attachment 324382 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=324382&action=review
> 
> >> Source/WebKit/PlatformGTK.cmake:1306
> >> +    COMMAND ${PERL_EXECUTABLE} ${WEBKIT_DIR}/Scripts/generate-forwarding-headers.pl --include-path ${WEBKIT_DIR} --include-path ${DERIVED_SOURCES_WEBKIT_DIR} --output ${FORWARDING_HEADERS_DIR} --platform gtk --platform soup
> > 
> > I don't think this works in a clean build, the derived sources might not have been generated yet. This is a known issue that we normally solve by explicitly including the required headers in the non-generated source file that includes the generated one.
> 
> Can this target not depend on the WebKit_DERIVED_SOURCES list, which would
> be the list of all source files generated via the generate-message-*.py
> scripts? Is there an existing bug covering this?

Maybe, I've never tried it. I don't remember if there's a bug for this...

[Zan Dobersek]

(In reply to Carlos Garcia Campos from comment #4)
> (In reply to Zan Dobersek from comment #3)
> > Comment on attachment 324382 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=324382&action=review
> > 
> > >> Source/WebKit/PlatformGTK.cmake:1306
> > >> +    COMMAND ${PERL_EXECUTABLE} ${WEBKIT_DIR}/Scripts/generate-forwarding-headers.pl --include-path ${WEBKIT_DIR} --include-path ${DERIVED_SOURCES_WEBKIT_DIR} --output ${FORWARDING_HEADERS_DIR} --platform gtk --platform soup
> > > 
> > > I don't think this works in a clean build, the derived sources might not have been generated yet. This is a known issue that we normally solve by explicitly including the required headers in the non-generated source file that includes the generated one.
> > 
> > Can this target not depend on the WebKit_DERIVED_SOURCES list, which would
> > be the list of all source files generated via the generate-message-*.py
> > scripts? Is there an existing bug covering this?
> 
> Maybe, I've never tried it. I don't remember if there's a bug for this...

Opened bug #178579.

[Zan Dobersek]

Comment on attachment 324382 [details]
Patch

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

> Source/WebKit/StorageProcess/StorageProcess.cpp:377
> -    IPC::Connection::Identifier connectionIdentifier = encodedConnectionIdentifier.releaseFileDescriptor();
> +    IPC::Connection::Identifier connectionIdentifier = encodedConnectionIdentifier.fileDescriptor();

We actually do have to release the file descriptor here. For that we need a mutable IPC::Attachment, so the didGetWorkerContextProcessConnection() method must expect an rvalue reference to the IPC::Attachment object, which is already casted as such in the IPC::handleMessage() implementation.

[Zan Dobersek]

Created attachment 324665 [details]
Patch

[Zan Dobersek]

Created attachment 324668 [details]
Patch for landing

Moves JSDOMPromise.cpp to the unified sources build process.

[Zan Dobersek]

Created attachment 324669 [details]
Patch for landing

Moves JSDOMPromise.cpp to the unified sources build process.

[Zan Dobersek]

Created attachment 324673 [details]
Patch

[Zan Dobersek]

Created attachment 324674 [details]
Patch

[Zan Dobersek]

(In reply to Zan Dobersek from comment #11)
> Created attachment 324674 [details]
> Patch

This finally builds now, and it's already reviewed. The r? flag was set by accident.

[Zan Dobersek]

Created attachment 324795 [details]
Patch for landing

[Zan Dobersek]

Comment on attachment 324795 [details]
Patch for landing

Clearing flags on attachment: 324795

Committed r223951: <https://trac.webkit.org/changeset/223951>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35568955>