Bug 108615 – [Soup] Wrap SoupSession by NetworkStorageSession

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Balazs Kelemen]

Soup should use this abstraction.

[Balazs Kelemen]

Created attachment 186080 [details]
Patch

[WebKit Review Bot]

Comment on attachment 186080 [details]
Patch

Attachment 186080 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16305884

[Early Warning System Bot]

Comment on attachment 186080 [details]
Patch

Attachment 186080 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16295857

[Alexey Proskuryakov]

Comment on attachment 186080 [details]
Patch

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

Cool!

> Source/WebCore/platform/network/soup/NetworkStorageSessionSoup.cpp:2
> + * Copyright (C) 2013 Apple Computer, Inc.  All rights reserved.

Did the Mac version say "Apple Computer"? The correct name is "Apple Inc."

> Source/WebKit2/WebProcess/WebCoreSupport/soup/WebFrameNetworkingContext.h:33
> +using namespace WebCore;

We normally don't put "using" into headers.

[Early Warning System Bot]

Comment on attachment 186080 [details]
Patch

Attachment 186080 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16295856

[WebKit Review Bot]

Comment on attachment 186080 [details]
Patch

Attachment 186080 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16305887

[Peter Beverloo (cr-android ews)]

Comment on attachment 186080 [details]
Patch

Attachment 186080 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/16309894

[Balazs Kelemen]

Created attachment 186258 [details]
Fixing builds, incorporate review comments.

[EFL EWS Bot]

Comment on attachment 186258 [details]
Fixing builds, incorporate review comments.

Attachment 186258 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16354502

[Chris Dumez]

Comment on attachment 186258 [details]
Fixing builds, incorporate review comments.

Did you forget to include NetworkStorageSessionSoup.cpp ?

[Build Bot]

Comment on attachment 186258 [details]
Fixing builds, incorporate review comments.

Attachment 186258 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16356484

[Build Bot]

Comment on attachment 186258 [details]
Fixing builds, incorporate review comments.

Attachment 186258 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16350527

[Balazs Kelemen]

Created attachment 186265 [details]
Adding new file

[Build Bot]

Comment on attachment 186265 [details]
Adding new file

Attachment 186265 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16360598

[Build Bot]

Comment on attachment 186265 [details]
Adding new file

Attachment 186265 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16357581

[Balazs Kelemen]

(In reply to comment #15)
> (From update of attachment 186265 [details])
> Attachment 186265 [details] did not pass win-ews (win):
> Output: http://queues.webkit.org/results/16357581

Does not seem to be my fault:
ImageDiffCG.cpp
1>..\cg\ImageDiffCG.cpp(37) : fatal error C1083: Cannot open include file: 'wtf/Platform.h': No such file or directory

I guess this bot is plain broken.

[Alexey Proskuryakov]

Comment on attachment 186265 [details]
Adding new file

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

> Source/WebCore/ChangeLog:6
> +        Reviewed by Alexey Proskuryakov.

Unsure why this is marked r?. Is there anything in particular you'd like me to have another look at?

[Balazs Kelemen]

(In reply to comment #17)
> (From update of attachment 186265 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186265&action=review
> 
> > Source/WebCore/ChangeLog:6
> > +        Reviewed by Alexey Proskuryakov.
> 
> Unsure why this is marked r?. Is there anything in particular you'd like me to have another look at?

Actually no, nothing important has changed, just I was needing the ews outputs.  I was believing that setting r+ myself is not a acceptable behavior :)

[Balazs Kelemen]

Comment on attachment 186265 [details]
Adding new file

Clearing flags on attachment: 186265

Committed r141749: <http://trac.webkit.org/changeset/141749>

[Balazs Kelemen]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

> I was believing that setting r+ myself is not a acceptable behavior :)

This is indeed not right.

However a patch that already has reviewer name in ChangeLog does not need any review flag, commit-queue won't complain (and so won't people if you manually commit a reviewed patch with build fixes).