Bug 130615 - More preparation for using Soup on Windows
Summary: More preparation for using Soup on Windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-21 13:55 PDT by Alex Christensen
Modified: 2014-03-31 09:30 PDT (History)
11 users (show)

See Also:


Attachments
Patch (28.41 KB, patch)
2014-03-21 14:07 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (16.89 KB, patch)
2014-03-27 10:58 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (16.89 KB, patch)
2014-03-27 14:55 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2014-03-21 13:55:18 PDT
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.
Comment 1 Alex Christensen 2014-03-21 14:07:32 PDT
Created attachment 227488 [details]
Patch
Comment 2 Carlos Garcia Campos 2014-03-22 01:27:23 PDT
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.
Comment 3 Alex Christensen 2014-03-27 10:58:35 PDT
Created attachment 227959 [details]
Patch
Comment 4 Alex Christensen 2014-03-27 11:02:09 PDT
(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.
Comment 5 Carlos Garcia Campos 2014-03-27 11:06:09 PDT
Can't you add the files conditionally to the build system instead? That's possible in all other build systems I think
Comment 6 Alex Christensen 2014-03-27 11:12:14 PDT
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.
Comment 7 Alex Christensen 2014-03-27 13:39:20 PDT
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?
Comment 8 Alex Christensen 2014-03-27 14:55:00 PDT
Created attachment 227985 [details]
Patch
Comment 9 Alex Christensen 2014-03-27 16:32:45 PDT
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.
Comment 10 Carlos Garcia Campos 2014-03-29 01:45:40 PDT
(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.
Comment 11 Alex Christensen 2014-03-29 11:50:48 PDT
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+?
Comment 12 Carlos Garcia Campos 2014-03-30 03:06:49 PDT
Comment on attachment 227985 [details]
Patch

Ok.
Comment 13 WebKit Commit Bot 2014-03-31 09:30:26 PDT
Comment on attachment 227985 [details]
Patch

Clearing flags on attachment: 227985

Committed r166506: <http://trac.webkit.org/changeset/166506>
Comment 14 WebKit Commit Bot 2014-03-31 09:30:33 PDT
All reviewed patches have been landed.  Closing bug.