| Summary: | [SOUP] Add SoupNetworkSession class to wrap a SoupSession | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||||
| Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | bunhere, cdumez, commit-queue, danw, eflews.bot, gustavo, gyuyoung.kim, mrobinson, rakuco, svillar | ||||||||||
| Priority: | P2 | Keywords: | Soup | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Carlos Garcia Campos
2014-01-11 06:14:52 PST
Created attachment 220928 [details]
Patch
Comment on attachment 220928 [details] Patch Attachment 220928 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/5803630930165760 Comment on attachment 220928 [details] Patch Attachment 220928 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/5057096894644224 Created attachment 220930 [details]
Try to fix EFL build
Comment on attachment 220930 [details] Try to fix EFL build Attachment 220930 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/5924820411744256 Created attachment 220932 [details]
Try to fix EFL build
Comment on attachment 220932 [details] Try to fix EFL build Attachment 220932 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/5367878714916864 Created attachment 220933 [details]
Try to fix EFL build
Comment on attachment 220933 [details]
Try to fix EFL build
Why not use NetworkStorageSessionSoup as the wrapper class? It feels like adding a new class just adds churn in this case, although I may be missing some life-cycle related issues?
Comment on attachment 220933 [details] Try to fix EFL build View in context: https://bugs.webkit.org/attachment.cgi?id=220933&action=review LGTM! Just wondering about the new class being required or if using the NetworkStorage would do it. > Source/WebKit/efl/ewk/ewk_cookies.cpp:47 > + g_object_unref(cookieJar); Any reason not use GRefPtr for this? (In reply to comment #10) > (From update of attachment 220933 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220933&action=review > > LGTM! Just wondering about the new class being required or if using the NetworkStorage would do it. Well, NetworkStorage is a cross-platform class in the end, this is just to wrap a SoupSession, with very specific soup code. > > Source/WebKit/efl/ewk/ewk_cookies.cpp:47 > > + g_object_unref(cookieJar); > > Any reason not use GRefPtr for this? I noticed that GRefPtr was not used anywhere in Source/WebKit/efl/ewk, and I thought it could be for some reason, so I used the explicit unref. (In reply to comment #11) > > LGTM! Just wondering about the new class being required or if using the NetworkStorage would do it. > > Well, NetworkStorage is a cross-platform class in the end, this is just to wrap a SoupSession, with very specific soup code. But it already has a soup-specific implementation, and soup-specific assessors, so we could use it, do you think there would be a downside? > > > Source/WebKit/efl/ewk/ewk_cookies.cpp:47 > > > + g_object_unref(cookieJar); > > > > Any reason not use GRefPtr for this? > > I noticed that GRefPtr was not used anywhere in Source/WebKit/efl/ewk, and I thought it could be for some reason, so I used the explicit unref. Makes sense. Comment on attachment 220933 [details]
Try to fix EFL build
OK, after our quick chat on IRC I think you're right about the NetworkStorageSession not being conceptually analog to the session, using NetworkingContext would make it more conceptually correct but it's per-frame, so I think your idea of a separate class makes sense. Thanks!
Committed r161890: <http://trac.webkit.org/changeset/161890> |