Bug 101395 - Clean up which storage cookie jar functions use
: Clean up which storage cookie jar functions use
: WebKit
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
  Show dependency treegraph
Reported: 2012-11-06 14:19 PST by
Modified: 2012-11-06 22:36 PST (History)

proposed patch (1.71 MB, patch)
2012-11-06 14:37 PST, Alexey Proskuryakov
no flags Review Patch | Details | Formatted Diff | Diff


You need to log in before you can comment on or make changes to this bug.

Description From 2012-11-06 14:19:26 PST
Some respect private browsing, and some don't. Furthermore, CF counterparts don't match Foundation ones.
------- Comment #1 From 2012-11-06 14:37:58 PST -------
Created an attachment (id=172654) [details]
proposed patch
------- Comment #2 From 2012-11-06 15:47:33 PST -------
(From update of attachment 172654 [details])
Attachment 172654 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14757165

New failing tests:
------- Comment #3 From 2012-11-06 16:07:42 PST -------
Seems super unlikely that this broke Chromium.
------- Comment #4 From 2012-11-06 16:19:43 PST -------
(From update of attachment 172654 [details])
Seems unobjectionable.
------- Comment #5 From 2012-11-06 16:42:15 PST -------
(From update of attachment 172654 [details])
Clearing flags on attachment: 172654

Committed r133694: <http://trac.webkit.org/changeset/133694>
------- Comment #6 From 2012-11-06 16:42:18 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #7 From 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.
------- Comment #8 From 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
------- Comment #9 From 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.
------- Comment #10 From 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+.
------- Comment #11 From 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.
------- Comment #12 From 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...