Bug 130615 – More preparation for using Soup on Windows

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Alex Christensen]

Similar to https://bugs.webkit.org/show_bug.cgi?id=130472

I've got a build compiling, but there is some problem with malloc and gmalloc.  I'm planning to upstream my changes to make debugging it have more localized changes.

[Alex Christensen]

Created attachment 227488 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 227488 [details]
Patch

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

> Source/WebCore/loader/soup/CachedRawResourceSoup.cpp:22
> +
> +#if USE(SOUP)
> +

Why do you need this? you are not supposed to build this file if not using soup

> Source/WebCore/loader/soup/SubresourceLoaderSoup.cpp:22
> +
> +#if USE(SOUP)
> +

Ditto.

> Source/WebCore/platform/soup/SharedBufferSoup.cpp:22
> +
> +#if USE(SOUP)
> +

Ditto.

> Source/WebCore/platform/soup/URLSoup.cpp:29
> +
> +#if USE(SOUP)
> +

Ditto.

[Alex Christensen]

Created attachment 227959 [details]
Patch

[Alex Christensen]

(In reply to comment #2)
> (From update of attachment 227488 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=227488&action=review
> 
> > Source/WebCore/loader/soup/CachedRawResourceSoup.cpp:22
> > +
> > +#if USE(SOUP)
> > +
> 
> Why do you need this? you are not supposed to build this file if not using soup
Like I said in https://bugs.webkit.org/show_bug.cgi?id=130472 I want to make a modified build of WinCairo without making a whole new build system in the Visual Studio project files.  I think this adds less clutter.

I uploaded an updated patch with COMPILER(MSVC) instead of defined _MSC_VER to be more webkit-like instead of glib-like.

[Carlos Garcia Campos]

Can't you add the files conditionally to the build system instead? That's possible in all other build systems I think

[Alex Christensen]

That's not possible with Visual Studio as we are using it.  Once CMake takes over that will be simple, but I think this is the best solution until then.

[Alex Christensen]

I suppose I could make another all-in-one cpp file, like NetworkingAllInOne.cpp or something that includes all the curl or soup files depending on USE(SOUP) or USE(CURL).  What would you think about that?

[Alex Christensen]

Created attachment 227985 [details]
Patch

[Alex Christensen]

Carlos, would you prefer I make an all-in-one file to conditionally include the soup files, or would you be ok with adding #if USE(SOUP) to the soup files.  I'm ok with either, but I've already started down this road, and I'd like to continue unless there is a reason to switch.

[Carlos Garcia Campos]

(In reply to comment #9)
> Carlos, would you prefer I make an all-in-one file to conditionally include the soup files, or would you be ok with adding #if USE(SOUP) to the soup files.  I'm ok with either, but I've already started down this road, and I'd like to continue unless there is a reason to switch.

I personally don't like the idea of adding more unnecessary #ifdefs to the code, but I'm not opposed if there's no other option, I don't want to block your work at all. If using an all in one file is more problematic for you, let's keep the ifdefs approach.

[Alex Christensen]

I think any approach would add some changes, and these ifdefs are pretty minimal. We could always change in the future if this becomes a problem. 

Could I get an r+?

[Carlos Garcia Campos]

Comment on attachment 227985 [details]
Patch

Ok.

[WebKit Commit Bot]

Comment on attachment 227985 [details]
Patch

Clearing flags on attachment: 227985

Committed r166506: <http://trac.webkit.org/changeset/166506>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.