WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
101395
Clean up which storage cookie jar functions use
https://bugs.webkit.org/show_bug.cgi?id=101395
Summary
Clean up which storage cookie jar functions use
Alexey Proskuryakov
Reported
2012-11-06 14:19:26 PST
Some respect private browsing, and some don't. Furthermore, CF counterparts don't match Foundation ones.
Attachments
proposed patch
(1.71 MB, patch)
2012-11-06 14:37 PST
,
Alexey Proskuryakov
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1
2012-11-06 14:37:58 PST
Created
attachment 172654
[details]
proposed patch
WebKit Review Bot
Comment 2
2012-11-06 15:47:33 PST
Comment on
attachment 172654
[details]
proposed patch
Attachment 172654
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/14757165
New failing tests: inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Alexey Proskuryakov
Comment 3
2012-11-06 16:07:42 PST
Seems super unlikely that this broke Chromium.
Brady Eidson
Comment 4
2012-11-06 16:19:43 PST
Comment on
attachment 172654
[details]
proposed patch Seems unobjectionable.
WebKit Review Bot
Comment 5
2012-11-06 16:42:15 PST
Comment on
attachment 172654
[details]
proposed patch Clearing flags on attachment: 172654 Committed
r133694
: <
http://trac.webkit.org/changeset/133694
>
WebKit Review Bot
Comment 6
2012-11-06 16:42:18 PST
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 7
2012-11-06 17:47:49 PST
I'm told that this broke Chromium Mac build. I'm not sure why WebKitSystemInterface changes can possibly have such an effect, someone from Chromium land will need to have a look. Please don't roll out.
Adam Barth
Comment 8
2012-11-06 21:30:28 PST
I've attempted a build fix in
http://trac.webkit.org/changeset/133721
I understand that you don't want the Chromium port using WebKitSystemInterface. The bug for that change is
https://code.google.com/p/chromium/issues/detail?id=101156
Adam Barth
Comment 9
2012-11-06 21:36:26 PST
That patch appears to have healed the chromium-mac build. I'm looking for someone to drive
http://crbug.com/101156
to completion.
Alexey Proskuryakov
Comment 10
2012-11-06 22:13:06 PST
Thank you Adam! Not sure why this fix works - does Chromium use an older copy of WKSI? It probably has to, because modern one is Lion+.
Nico Weber
Comment 11
2012-11-06 22:17:27 PST
(In reply to
comment #10
)
> Thank you Adam! Not sure why this fix works - does Chromium use an older copy of WKSI? It probably has to, because modern one is Lion+.
Yes, chromium uses an old version (
http://crbug.com/101387
). From what I understand, that's what weinig suggested on some private thread (that I wasn't on myself) between avi, darin (adler), fishd, msj, and a few others.
Alexey Proskuryakov
Comment 12
2012-11-06 22:36:23 PST
I was not aware of the setup either. I still don't fully understand it, neither the technical details, nor whether there is any kind of process that should prevent breaking the build. Does Chromium link to the oldest or the newest WKSI static library? Or are they converted to dynamic libraries, and the one that matches current OS is used? Looking at this specific build failure, I think that forking WebCoreSupport/WebSystemInterface.mm would have prevented it. There is no reason to try to initialize symbols that are not in your copy of WKSI, and that you do not need at all. This won't resolve all issues though, as (1) the code in WKSI is not guaranteed to work in future OS versions, (2) we can always add more WKSI dependencies in WebCore, and I'm sure that there are (3), (4) and so on...
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