Bug 108615

Summary: [Soup] Wrap SoupSession by NetworkStorageSession
Product: WebKit Reporter: Balazs Kelemen <kbalazs>
Component: PlatformAssignee: Balazs Kelemen <kbalazs>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, danw, dglazkov, gustavo, gyuyoung.kim, japhet, jesus, kenneth, mrobinson, peter+ews, philn, rakuco, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 108832, 108613    
Attachments:
Description Flags
Patch
none
Fixing builds, incorporate review comments.
none
Adding new file none

Description Balazs Kelemen 2013-02-01 04:32:08 PST
Soup should use this abstraction.
Comment 1 Balazs Kelemen 2013-02-01 10:57:03 PST
Created attachment 186080 [details]
Patch
Comment 2 WebKit Review Bot 2013-02-01 11:02:11 PST
Comment on attachment 186080 [details]
Patch

Attachment 186080 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16305884
Comment 3 Early Warning System Bot 2013-02-01 11:02:55 PST
Comment on attachment 186080 [details]
Patch

Attachment 186080 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16295857
Comment 4 Alexey Proskuryakov 2013-02-01 11:03:26 PST
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.
Comment 5 Early Warning System Bot 2013-02-01 11:03:30 PST
Comment on attachment 186080 [details]
Patch

Attachment 186080 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16295856
Comment 6 WebKit Review Bot 2013-02-01 11:03:39 PST
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
Comment 7 Peter Beverloo (cr-android ews) 2013-02-01 11:08:14 PST
Comment on attachment 186080 [details]
Patch

Attachment 186080 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/16309894
Comment 8 Balazs Kelemen 2013-02-03 04:39:21 PST
Created attachment 186258 [details]
Fixing builds, incorporate review comments.
Comment 9 EFL EWS Bot 2013-02-03 04:42:25 PST
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
Comment 10 Chris Dumez 2013-02-03 05:00:14 PST
Comment on attachment 186258 [details]
Fixing builds, incorporate review comments.

Did you forget to include NetworkStorageSessionSoup.cpp ?
Comment 11 Build Bot 2013-02-03 05:41:48 PST
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
Comment 12 Build Bot 2013-02-03 06:42:03 PST
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
Comment 13 Balazs Kelemen 2013-02-03 09:04:23 PST
Created attachment 186265 [details]
Adding new file
Comment 14 Build Bot 2013-02-03 10:10:04 PST
Comment on attachment 186265 [details]
Adding new file

Attachment 186265 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16360598
Comment 15 Build Bot 2013-02-03 11:14:34 PST
Comment on attachment 186265 [details]
Adding new file

Attachment 186265 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16357581
Comment 16 Balazs Kelemen 2013-02-03 11:19:31 PST
(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.
Comment 17 Alexey Proskuryakov 2013-02-03 19:22:32 PST
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?
Comment 18 Balazs Kelemen 2013-02-04 01:31:07 PST
(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 :)
Comment 19 Balazs Kelemen 2013-02-04 01:35:06 PST
Comment on attachment 186265 [details]
Adding new file

Clearing flags on attachment: 186265

Committed r141749: <http://trac.webkit.org/changeset/141749>
Comment 20 Balazs Kelemen 2013-02-04 01:35:14 PST
All reviewed patches have been landed.  Closing bug.
Comment 21 Alexey Proskuryakov 2013-02-04 08:27:25 PST
> 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).