121423 2013-09-16 04:29:03 -0700 CookieStorageShim should be PLATFORM(MAC) guarded 2013-09-27 16:39:22 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 110141 1 ossy ossy andersca ap beidson benjamin commit-queue darin eric.carlson jer.noble kbalazs ossy psolanki oldest_to_newest 929629 0 ossy 2013-09-16 04:29:03 -0700 CookieStorageShim is used by only Mac, so all of its use should be guarded. 929630 1 211761 ossy 2013-09-16 04:31:46 -0700 Created attachment 211761 Patch 930262 2 211761 ap 2013-09-17 09:33:03 -0700 Comment on attachment 211761 Patch Shouldn't this code be moved to platformInitializeWebProcess instead of adding ifdefs? I'm not quite sure about iOS implications, but as long as the patch doesn't change behavior, it should be fine to land. The current one doesn't change behavior, and one that changes platformInitializeWebProcess shouldn't either. 930880 3 ossy 2013-09-19 03:24:07 -0700 I checked the git history and I found this CookieStorageShim was introduced in http://trac.webkit.org/changeset/149074. The commit log mentioned this change is related to AVFoundation and CFNetwork. I have no idea if the Linux ports will need a similar implementation in the future or not. Now a buildable and more or less working NetworkProcess is my first priority. Bugfixes can be done after that. (In reply to comment #2) > (From update of attachment 211761 [details]) > Shouldn't this code be moved to platformInitializeWebProcess instead of adding ifdefs? > > I'm not quite sure about iOS implications, but as long as the patch doesn't change behavior, it should be fine to land. The current one doesn't change behavior, and one that changes platformInitializeWebProcess shouldn't either. I don't know anything about the iOS implications, because iOS port isn't fully upstreamed yet. Otherwise I thought there isn't WebKit2 on iOS at all - https://lists.webkit.org/pipermail/webkit-dev/2013-February/023735.html Of course it might be changed in the last half year. :) I'm not sure if it is safe to simple move this code to the platformInitializeWebProcess. There is an ensureNetworkProcessConnection(); call before the CookieStorageShim::shared().initialize(); call now. Is it really needed or not for CookieStorageShim initialization? So I still think that the safest way for the fix is to leave this code here and simple adding the PLATFORM(MAC) ifdef guard. 932915 4 ossy 2013-09-25 02:51:10 -0700 It seems I wasn't up-to-date, WebKit2 on IOS already exists: - http://trac.webkit.org/changeset/141025/trunk/Source/WebKit2/WebProcess/WebCoreSupport/mac/WebSystemInterface.mm - http://trac.webkit.org/changeset/156350 But I don't think if I should now if you use CookieStorageShim in the secret IOS branch or not. In the public WebKit trunk it is used by only PLATFORM(MAC), so the proposed guard is correct. 933045 5 darin 2013-09-25 09:39:04 -0700 PLATFORM(MAC) is currently and has always been true for the iOS port. 933888 6 ossy 2013-09-27 04:18:40 -0700 (In reply to comment #5) > PLATFORM(MAC) is currently and has always been true for the iOS port. In this case it should be safe to land it. Can I have an r+ for it please? 934077 7 darin 2013-09-27 14:07:01 -0700 Anders, do you have any objection to doing this through a PLATFORM(MAC) conditional? Is this Mac-specific? 934141 8 211761 commit-queue 2013-09-27 16:39:20 -0700 Comment on attachment 211761 Patch Clearing flags on attachment: 211761 Committed r156589: <http://trac.webkit.org/changeset/156589> 934142 9 commit-queue 2013-09-27 16:39:22 -0700 All reviewed patches have been landed. Closing bug. 211761 2013-09-16 04:31:46 -0700 2013-09-27 16:39:20 -0700 Patch bug-121423-20130916133144.patch text/plain 1642 ossy U3VidmVyc2lvbiBSZXZpc2lvbjogMTU1ODQ1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggNDI5NDljOTBhNjU5ZjI3 ZDRkNjNkYzZjNzVkYTRkMGZhYTkwYWFiNi4uOWQ0NWZiYmFiOGQ4ZDk2MTFkODA4NTJhZWUxOWUy OGMzMzM1NWU3MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDEzLTA5LTE2ICBDc2Fi YSBPc3p0cm9nb27DoWMgIDxvc3N5QHdlYmtpdC5vcmc+CisKKyAgICAgICAgQ29va2llU3RvcmFn ZVNoaW0gc2hvdWxkIGJlIFBMQVRGT1JNKE1BQykgZ3VhcmRlZAorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTIxNDIzCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgT3JpZ2luYWwgcGF0Y2ggYnkgS3dhbmcgWXVs IFNlbyA8c2t5dWxAY29tcGFueTEwMC5jb20+LgorCisgICAgICAgICogV2ViUHJvY2Vzcy9XZWJQ cm9jZXNzLmNwcDoKKyAgICAgICAgKFdlYktpdDo6V2ViUHJvY2Vzczo6aW5pdGlhbGl6ZVdlYlBy b2Nlc3MpOgorCiAyMDEzLTA5LTE1ICBHdXN0YXZvIE5vcm9uaGEgU2lsdmEgIDxnbnNAZ25vbWUu b3JnPgogCiAgICAgICAgIFVucmV2aWV3ZWQgbWFrZSBkaXN0Y2hlY2sgZml4LgpkaWZmIC0tZ2l0 IGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQcm9jZXNzLmNwcCBiL1NvdXJjZS9XZWJL aXQyL1dlYlByb2Nlc3MvV2ViUHJvY2Vzcy5jcHAKaW5kZXggZjI2OTA5NGQ0MWFhMGQxNmYwYTdl NzA2NjE2NDkxODkzZGUyYjY4ZC4uNzAyOWY5ZWI4YmM5ZjJkNDdjNDY4NDA3ZjhkNDA3MTQ3ODFl YzE3NyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJQcm9jZXNzLmNw cAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL1dlYlByb2Nlc3MuY3BwCkBAIC05MCw3 ICs5MCw5IEBACiAjZW5kaWYKIAogI2lmIEVOQUJMRShORVRXT1JLX1BST0NFU1MpCisjaWYgUExB VEZPUk0oTUFDKQogI2luY2x1ZGUgIkNvb2tpZVN0b3JhZ2VTaGltLmgiCisjZW5kaWYKICNpbmNs dWRlICJOZXR3b3JrUHJvY2Vzc0Nvbm5lY3Rpb24uaCIKICNlbmRpZgogCkBAIC0zNTEsOSArMzUz LDExIEBAIHZvaWQgV2ViUHJvY2Vzczo6aW5pdGlhbGl6ZVdlYlByb2Nlc3MoY29uc3QgV2ViUHJv Y2Vzc0NyZWF0aW9uUGFyYW1ldGVycyYgcGFyYW1lCiAgICAgbV91c2VzTmV0d29ya1Byb2Nlc3Mg PSBwYXJhbWV0ZXJzLnVzZXNOZXR3b3JrUHJvY2VzczsKICAgICBlbnN1cmVOZXR3b3JrUHJvY2Vz c0Nvbm5lY3Rpb24oKTsKIAorI2lmIFBMQVRGT1JNKE1BQykKICAgICBpZiAodXNlc05ldHdvcmtQ cm9jZXNzKCkpCiAgICAgICAgIENvb2tpZVN0b3JhZ2VTaGltOjpzaGFyZWQoKS5pbml0aWFsaXpl KCk7CiAjZW5kaWYKKyNlbmRpZgogICAgIHNldFRlcm1pbmF0aW9uVGltZW91dChwYXJhbWV0ZXJz LnRlcm1pbmF0aW9uVGltZW91dCk7CiAKICAgICByZXNldFBsdWdJbkF1dG9TdGFydE9yaWdpbkhh c2hlcyhwYXJhbWV0ZXJzLnBsdWdJbkF1dG9TdGFydE9yaWdpbkhhc2hlcyk7Cg==