WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 130615
More preparation for using Soup on Windows
https://bugs.webkit.org/show_bug.cgi?id=130615
Summary
More preparation for using Soup on Windows
Alex Christensen
Reported
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.
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
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Alex Christensen
Comment 1
2014-03-21 14:07:32 PDT
Created
attachment 227488
[details]
Patch
Carlos Garcia Campos
Comment 2
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.
Alex Christensen
Comment 3
2014-03-27 10:58:35 PDT
Created
attachment 227959
[details]
Patch
Alex Christensen
Comment 4
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.
Carlos Garcia Campos
Comment 5
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
Alex Christensen
Comment 6
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.
Alex Christensen
Comment 7
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?
Alex Christensen
Comment 8
2014-03-27 14:55:00 PDT
Created
attachment 227985
[details]
Patch
Alex Christensen
Comment 9
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.
Carlos Garcia Campos
Comment 10
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.
Alex Christensen
Comment 11
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+?
Carlos Garcia Campos
Comment 12
2014-03-30 03:06:49 PDT
Comment on
attachment 227985
[details]
Patch Ok.
WebKit Commit Bot
Comment 13
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
>
WebKit Commit Bot
Comment 14
2014-03-31 09:30:33 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug