WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
168229
CookieManager only works with the default session
https://bugs.webkit.org/show_bug.cgi?id=168229
Summary
CookieManager only works with the default session
Carlos Garcia Campos
Reported
2017-02-13 04:59:42 PST
It should be possible to use the cookie manager also for ephemeral sessions.
Attachments
Patch
(34.88 KB, patch)
2017-02-13 05:07 PST
,
Carlos Garcia Campos
achristensen
: review+
Details
Formatted Diff
Diff
Patch for landing
(35.15 KB, patch)
2017-02-14 01:39 PST
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2017-02-13 05:07:30 PST
Created
attachment 301344
[details]
Patch
WebKit Commit Bot
Comment 2
2017-02-13 05:10:16 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See
http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
WebKit Commit Bot
Comment 3
2017-02-13 05:10:27 PST
Attachment 301344
[details]
did not pass style-queue: ERROR: Source/WebCore/platform/network/CookieStorage.h:34: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/mac/CookieStorageMac.mm:34: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/mac/CookieStorageMac.mm:38: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/mac/CookieStorageMac.mm:56: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/mac/CookieStorageMac.mm:75: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.h:68: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.h:77: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.h:113: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:44: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:46: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:69: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp:96: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp:136: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/soup/CookieStorageSoup.cpp:31: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/soup/CookieStorageSoup.cpp:33: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/soup/CookieStorageSoup.cpp:43: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 16 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Michael Catanzaro
Comment 4
2017-02-13 17:02:18 PST
Comment on
attachment 301344
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=301344&action=review
Nice work! I trust you'll find an owner to review it. Any reason you used std::function instead of the preferred WTF::Function?
> Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:46 > + static NeverDestroyed<HashMap<CFHTTPCookieStorageRef, std::function<void ()>>> map;
You need to fix Windows: C:\cygwin\home\buildbot\WebKit\Source\WebCore\platform\network\cf\CookieStorageCFNet.cpp(46): error C2079: 'map' uses undefined class 'WTF::NeverDestroyed<WTF::HashMap<CFHTTPCookieStorageRef,std::function<void (void)>,WTF::PtrHash<P *>,WTF::HashTraits<KeyArg>,WTF::HashTraits<MappedArg>>>' [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] with [ P=OpaqueCFHTTPCookieStorage, KeyArg=CFHTTPCookieStorageRef, MappedArg=std::function<void (void)> ] Probably a missing #include?
Alex Christensen
Comment 5
2017-02-13 17:27:38 PST
Comment on
attachment 301344
[details]
Patch I like the direction this code is going in. Definitely fix the Windows build. WTF::Functions are not copyable, and that seems preferable in this case. Can this be tested?
Alex Christensen
Comment 6
2017-02-13 17:29:13 PST
(In reply to
comment #5
)
> Can this be tested?
Aha! This is definitely related to
https://bugs.webkit.org/show_bug.cgi?id=168230
Alex Christensen
Comment 7
2017-02-13 17:34:36 PST
Comment on
attachment 301344
[details]
Patch r=me, fix Windows. Let's get rid of CookiesStrategy, too!
Michael Catanzaro
Comment 8
2017-02-13 18:02:05 PST
(In reply to
comment #6
)
> (In reply to
comment #5
) > > Can this be tested? > Aha! This is definitely related to >
https://bugs.webkit.org/show_bug.cgi?id=168230
Yeah, it's because we just exposed ephemeral mode. See
https://lists.webkit.org/pipermail/webkit-gtk/2017-February/002911.html
. Thanks for reviewing it quickly, because we want to freeze API for the next release now!
Carlos Garcia Campos
Comment 9
2017-02-13 23:44:52 PST
(In reply to
comment #4
)
> Comment on
attachment 301344
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=301344&action=review
> > Nice work! I trust you'll find an owner to review it. > > Any reason you used std::function instead of the preferred WTF::Function?
Because a non copyable function can't be used as a value of a HashMap.
> > Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:46 > > + static NeverDestroyed<HashMap<CFHTTPCookieStorageRef, std::function<void ()>>> map; > > You need to fix Windows: > > C: > \cygwin\home\buildbot\WebKit\Source\WebCore\platform\network\cf\CookieStorage > CFNet.cpp(46): error C2079: 'map' uses undefined class > 'WTF::NeverDestroyed<WTF::HashMap<CFHTTPCookieStorageRef,std::function<void > (void)>,WTF::PtrHash<P > *>,WTF::HashTraits<KeyArg>,WTF::HashTraits<MappedArg>>>' > [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\WebCore\WebCore. > vcxproj] > with > [ > P=OpaqueCFHTTPCookieStorage, > KeyArg=CFHTTPCookieStorageRef, > MappedArg=std::function<void (void)> > ] > > Probably a missing #include?
Yes.
Alex Christensen
Comment 10
2017-02-13 23:46:11 PST
(In reply to
comment #9
)
> > Any reason you used std::function instead of the preferred WTF::Function? > > Because a non copyable function can't be used as a value of a HashMap.
You're right :(
Carlos Garcia Campos
Comment 11
2017-02-13 23:46:31 PST
(In reply to
comment #5
)
> Comment on
attachment 301344
[details]
> Patch > > I like the direction this code is going in. Definitely fix the Windows > build. WTF::Functions are not copyable, and that seems preferable in this > case. Can this be tested?
I started using Function but had to change it to std::function to be able to use it in HashMap value.
Carlos Garcia Campos
Comment 12
2017-02-14 01:39:32 PST
Created
attachment 301478
[details]
Patch for landing
WebKit Commit Bot
Comment 13
2017-02-14 02:09:33 PST
Attachment 301478
[details]
did not pass style-queue: ERROR: Source/WebCore/platform/network/CookieStorage.h:34: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/mac/CookieStorageMac.mm:34: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/mac/CookieStorageMac.mm:38: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/mac/CookieStorageMac.mm:56: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/mac/CookieStorageMac.mm:75: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.h:68: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.h:77: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.h:113: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:46: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:48: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:71: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp:96: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/UIProcess/WebCookieManagerProxy.cpp:136: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/soup/CookieStorageSoup.cpp:31: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/soup/CookieStorageSoup.cpp:33: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/platform/network/soup/CookieStorageSoup.cpp:43: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 16 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Carlos Garcia Campos
Comment 14
2017-02-14 02:45:02 PST
Committed
r212283
: <
http://trac.webkit.org/changeset/212283
>
Brady Eidson
Comment 15
2017-02-14 10:39:14 PST
(In reply to
comment #11
)
> (In reply to
comment #5
) > > Comment on
attachment 301344
[details]
> > Patch > > > > I like the direction this code is going in. Definitely fix the Windows > > build. WTF::Functions are not copyable, and that seems preferable in this > > case. Can this be tested? > > I started using Function but had to change it to std::function to be able to > use it in HashMap value.
Nothing about a Function precludes it from being a HashMap value; We use Functions in HashMap values all the time. What was preventing you from doing so? Could you please revisit this?
Brady Eidson
Comment 16
2017-02-14 10:48:58 PST
(In reply to
comment #15
)
> (In reply to
comment #11
) > > (In reply to
comment #5
) > > > Comment on
attachment 301344
[details]
> > > Patch > > > > > > I like the direction this code is going in. Definitely fix the Windows > > > build. WTF::Functions are not copyable, and that seems preferable in this > > > case. Can this be tested? > > > > I started using Function but had to change it to std::function to be able to > > use it in HashMap value. > > Nothing about a Function precludes it from being a HashMap value; We use > Functions in HashMap values all the time.
For hints, search WebKit2 for: `HashMap<uint64_t, Function`
Carlos Garcia Campos
Comment 17
2017-02-14 22:29:32 PST
(In reply to
comment #15
)
> (In reply to
comment #11
) > > (In reply to
comment #5
) > > > Comment on
attachment 301344
[details]
> > > Patch > > > > > > I like the direction this code is going in. Definitely fix the Windows > > > build. WTF::Functions are not copyable, and that seems preferable in this > > > case. Can this be tested? > > > > I started using Function but had to change it to std::function to be able to > > use it in HashMap value. > > Nothing about a Function precludes it from being a HashMap value; We use > Functions in HashMap values all the time. > > What was preventing you from doing so? > > Could you please revisit this?
You are right, now I remember exactly the issue. You can indeed use Function as a value of a HshMap, but you can't get it, because that always requires a copy, at least with the current HashMap implementation, I'm not sure if it's possible to implement a git without copying.
Carlos Garcia Campos
Comment 18
2017-02-14 22:31:51 PST
(In reply to
comment #16
)
> (In reply to
comment #15
) > > (In reply to
comment #11
) > > > (In reply to
comment #5
) > > > > Comment on
attachment 301344
[details]
> > > > Patch > > > > > > > > I like the direction this code is going in. Definitely fix the Windows > > > > build. WTF::Functions are not copyable, and that seems preferable in this > > > > case. Can this be tested? > > > > > > I started using Function but had to change it to std::function to be able to > > > use it in HashMap value. > > > > Nothing about a Function precludes it from being a HashMap value; We use > > Functions in HashMap values all the time. > > For hints, search WebKit2 for: > `HashMap<uint64_t, Function`
I did it when I found the issue, and all those were cases where function is added once and then taken, like completion handlers. In that case there's no problem because take doesn't need to copy, it just moves. Here we need to set once and get multiple times. We could take and set on every get, but I don't think it's worth it.
Michael Catanzaro
Comment 19
2017-02-15 09:50:38 PST
I guess it caused
bug #168375
.
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