129349 2014-02-25 16:59:08 -0800 Assertion failure at CachedResource.h:196: ASSERT(!m_purgeableData) 2014-02-27 11:06:24 -0800 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 psolanki psolanki commit-queue joepeck psolanki oldest_to_newest 984549 0 psolanki 2014-02-25 16:59:08 -0800 This assertion triggers frequently when launching Safari/loading URLs. 984551 1 225199 psolanki 2014-02-25 17:02:02 -0800 Created attachment 225199 Patch 984552 2 psolanki 2014-02-25 17:02:39 -0800 <rdar://problem/14871837> 984553 3 225200 psolanki 2014-02-25 17:03:21 -0800 Created attachment 225200 Patch 984782 4 225200 darin 2014-02-26 10:01:22 -0800 Comment on attachment 225200 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225200&action=review > Source/WebKit/mac/ChangeLog:11 > + The code for clearing out memory mapped notification callbacks is only needed when loading > + PDFs. And in such cases, we always have dataSourceDelegate object. So make this code > + conditional on its presence so that we don't trigger the assert for non-PDF main resources. Maybe the change is OK, but the code change itself is completely mysterious. There’s a null check of dataSourceDelegate, but nothing makes clear why it’s OK to skip the code below if the delegate is null. I think you need some kind of comment. 984809 5 psolanki 2014-02-26 11:02:32 -0800 (In reply to comment #4) > (From update of attachment 225200 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=225200&action=review > > > Source/WebKit/mac/ChangeLog:11 > > + The code for clearing out memory mapped notification callbacks is only needed when loading > > + PDFs. And in such cases, we always have dataSourceDelegate object. So make this code > > + conditional on its presence so that we don't trigger the assert for non-PDF main resources. > > Maybe the change is OK, but the code change itself is completely mysterious. There’s a null check of dataSourceDelegate, but nothing makes clear why it’s OK to skip the code below if the delegate is null. I think you need some kind of comment. Yeah, I agree. I'll add comments in the change. I think Joe and I are leaning towards removing this whole disk image cache code path now that we have CFNetwork disk cache. 984810 6 psolanki 2014-02-26 11:02:32 -0800 (In reply to comment #4) > (From update of attachment 225200 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=225200&action=review > > > Source/WebKit/mac/ChangeLog:11 > > + The code for clearing out memory mapped notification callbacks is only needed when loading > > + PDFs. And in such cases, we always have dataSourceDelegate object. So make this code > > + conditional on its presence so that we don't trigger the assert for non-PDF main resources. > > Maybe the change is OK, but the code change itself is completely mysterious. There’s a null check of dataSourceDelegate, but nothing makes clear why it’s OK to skip the code below if the delegate is null. I think you need some kind of comment. Yeah, I agree. I'll add comments in the change. I think Joe and I are leaning towards removing this whole disk image cache code path now that we have CFNetwork disk cache. 984856 7 225284 psolanki 2014-02-26 12:49:38 -0800 Created attachment 225284 Patch 985246 8 225284 joepeck 2014-02-27 10:44:31 -0800 Comment on attachment 225284 Patch r=me, This is a harmless addition to avoid an assert we know about and can't easily fix. We want to just remove DiskImageCache altogether eventually. 985261 9 225284 commit-queue 2014-02-27 11:06:21 -0800 Comment on attachment 225284 Patch Clearing flags on attachment: 225284 Committed r164817: <http://trac.webkit.org/changeset/164817> 985262 10 commit-queue 2014-02-27 11:06:24 -0800 All reviewed patches have been landed. Closing bug. 225199 2014-02-25 17:02:02 -0800 2014-02-25 17:03:19 -0800 Patch bug-129349-20140225170201.patch text/plain 1797 psolanki U3VidmVyc2lvbiBSZXZpc2lvbjogMTY0NTkxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L21h Yy9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0L21hYy9DaGFuZ2VMb2cKaW5kZXggMzNkNjcxN2Uz ODQzMGJiY2FkZmRkYTQyOTBmZTA0ZTQwNjU3YzBmNy4uZGI3MWRlM2RlMjU5ZGQyYzgyMDA3Nzg2 MmUxMDAwZDkyYzZiYzlkNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9tYWMvQ2hhbmdlTG9n CisrKyBiL1NvdXJjZS9XZWJLaXQvbWFjL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE0 LTAyLTI1ICBQcmF0aWsgU29sYW5raSAgPHBzb2xhbmtpQGFwcGxlLmNvbT4KKworICAgICAgICBB c3NlcnRpb24gZmFpbHVyZSBhdCBDYWNoZWRSZXNvdXJjZS5oOjE5NjogQVNTRVJUKCFtX3B1cmdl YWJsZURhdGEpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xMjkzNDkKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBUaGUgY29kZSBmb3IgY2xlYXJpbmcgb3V0IG1lbW9yeSBtYXBwZWQgbm90aWZpY2F0aW9uIGNh bGxiYWNrcyBpcyBvbmx5IG5lZWRlZCB3aGVuIGxvYWRpbmcKKyAgICAgICAgUERGcy4gQW5kIGlu IHN1Y2ggY2FzZXMsIHdlIGFsd2F5cyBoYXZlIGRhdGFTb3VyY2VEZWxlZ2F0ZSBvYmplY3QuIFNv IG1ha2UgdGhpcyBjb2RlCisgICAgICAgIGNvbmRpdGlvbmFsIG9uIGl0cyBwcmVzZW5jZSBzbyB0 aGF0IHdlIGRvbid0IHRyaWdnZXIgdGhlIGFzc2VydCBmb3Igbm9uLVBERiBtYWluIHJlc291cmNl cy4KKworICAgICAgICAqIFdlYlZpZXcvV2ViRGF0YVNvdXJjZS5tbToKKyAgICAgICAgKC1bV2Vi RGF0YVNvdXJjZSBkZWFsbG9jXSk6CisKIDIwMTQtMDItMjQgIFRhbWFzIEdlcmdlbHkgIDx0Z2Vy Z2VseS51LXN6ZWdlZEBwYXJ0bmVyLnNhbXN1bmcuY29tPgogCiAgICAgICAgIENvZGUgY2xlYW51 cDogcmVtb3ZlIGxlZnRvdmVyIEVOQUJMRShXT1JLRVJTKSBtYWNyb3MgYW5kIHN1cHBvcnQuCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L21hYy9XZWJWaWV3L1dlYkRhdGFTb3VyY2UubW0gYi9T b3VyY2UvV2ViS2l0L21hYy9XZWJWaWV3L1dlYkRhdGFTb3VyY2UubW0KaW5kZXggYTk1ZmM1ZWVi MzkyYjUyZDhmYWRkMDEyMjE2NzVjMThhYjA5NTk4MC4uZjkwZDdkODVjZDE0N2VlN2YwOTEyOGVh NzllNTliMjY4ZTU0ZWU2MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9tYWMvV2ViVmlldy9X ZWJEYXRhU291cmNlLm1tCisrKyBiL1NvdXJjZS9XZWJLaXQvbWFjL1dlYlZpZXcvV2ViRGF0YVNv dXJjZS5tbQpAQCAtNDgxLDcgKzQ4MSw3IEBAIHN0YXRpYyBpbmxpbmUgdm9pZCBhZGRUeXBlc0Zy b21DbGFzcyhOU011dGFibGVEaWN0aW9uYXJ5ICphbGxUeXBlcywgQ2xhc3Mgb2JqQ0NsCiAgICAg ICAgIC0tV2ViRGF0YVNvdXJjZUNvdW50OwogCiAjaWYgRU5BQkxFKERJU0tfSU1BR0VfQ0FDSEUp ICYmIFBMQVRGT1JNKElPUykKLSAgICBpZiAoX3ByaXZhdGUpIHsKKyAgICBpZiAoX3ByaXZhdGUg JiYgW3NlbGYgZGF0YVNvdXJjZURlbGVnYXRlXSkgewogICAgICAgICBSZWZQdHI8UmVzb3VyY2VC dWZmZXI+IG1haW5SZXNvdXJjZUJ1ZmZlciA9IHRvUHJpdmF0ZShfcHJpdmF0ZSktPmxvYWRlci0+ bWFpblJlc291cmNlRGF0YSgpOwogICAgICAgICBpZiAobWFpblJlc291cmNlQnVmZmVyKSB7CiAg ICAgICAgICAgICBSZWZQdHI8U2hhcmVkQnVmZmVyPiBtYWluUmVzb3VyY2VEYXRhID0gbWFpblJl c291cmNlQnVmZmVyLT5zaGFyZWRCdWZmZXIoKTsK 225200 2014-02-25 17:03:21 -0800 2014-02-26 12:49:37 -0800 Patch bug-129349-20140225170320.patch text/plain 1832 psolanki U3VidmVyc2lvbiBSZXZpc2lvbjogMTY0NTkxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L21h Yy9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0L21hYy9DaGFuZ2VMb2cKaW5kZXggMzNkNjcxN2Uz ODQzMGJiY2FkZmRkYTQyOTBmZTA0ZTQwNjU3YzBmNy4uNjNlY2Q4ZmZjMzMyYWQxMmJkOTgzMjg2 ZmFlNDgyNTdkMmI0NjU1YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9tYWMvQ2hhbmdlTG9n CisrKyBiL1NvdXJjZS9XZWJLaXQvbWFjL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE0 LTAyLTI1ICBQcmF0aWsgU29sYW5raSAgPHBzb2xhbmtpQGFwcGxlLmNvbT4KKworICAgICAgICBB c3NlcnRpb24gZmFpbHVyZSBhdCBDYWNoZWRSZXNvdXJjZS5oOjE5NjogQVNTRVJUKCFtX3B1cmdl YWJsZURhdGEpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xMjkzNDkKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzE0ODcxODM3PgorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBjb2RlIGZvciBjbGVhcmlu ZyBvdXQgbWVtb3J5IG1hcHBlZCBub3RpZmljYXRpb24gY2FsbGJhY2tzIGlzIG9ubHkgbmVlZGVk IHdoZW4gbG9hZGluZworICAgICAgICBQREZzLiBBbmQgaW4gc3VjaCBjYXNlcywgd2UgYWx3YXlz IGhhdmUgZGF0YVNvdXJjZURlbGVnYXRlIG9iamVjdC4gU28gbWFrZSB0aGlzIGNvZGUKKyAgICAg ICAgY29uZGl0aW9uYWwgb24gaXRzIHByZXNlbmNlIHNvIHRoYXQgd2UgZG9uJ3QgdHJpZ2dlciB0 aGUgYXNzZXJ0IGZvciBub24tUERGIG1haW4gcmVzb3VyY2VzLgorCisgICAgICAgICogV2ViVmll dy9XZWJEYXRhU291cmNlLm1tOgorICAgICAgICAoLVtXZWJEYXRhU291cmNlIGRlYWxsb2NdKToK KwogMjAxNC0wMi0yNCAgVGFtYXMgR2VyZ2VseSAgPHRnZXJnZWx5LnUtc3plZ2VkQHBhcnRuZXIu c2Ftc3VuZy5jb20+CiAKICAgICAgICAgQ29kZSBjbGVhbnVwOiByZW1vdmUgbGVmdG92ZXIgRU5B QkxFKFdPUktFUlMpIG1hY3JvcyBhbmQgc3VwcG9ydC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJL aXQvbWFjL1dlYlZpZXcvV2ViRGF0YVNvdXJjZS5tbSBiL1NvdXJjZS9XZWJLaXQvbWFjL1dlYlZp ZXcvV2ViRGF0YVNvdXJjZS5tbQppbmRleCBhOTVmYzVlZWIzOTJiNTJkOGZhZGQwMTIyMTY3NWMx OGFiMDk1OTgwLi5mOTBkN2Q4NWNkMTQ3ZWU3ZjA5MTI4ZWE3OWU1OWIyNjhlNTRlZTYxIDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViS2l0L21hYy9XZWJWaWV3L1dlYkRhdGFTb3VyY2UubW0KKysrIGIv U291cmNlL1dlYktpdC9tYWMvV2ViVmlldy9XZWJEYXRhU291cmNlLm1tCkBAIC00ODEsNyArNDgx LDcgQEAgc3RhdGljIGlubGluZSB2b2lkIGFkZFR5cGVzRnJvbUNsYXNzKE5TTXV0YWJsZURpY3Rp b25hcnkgKmFsbFR5cGVzLCBDbGFzcyBvYmpDQ2wKICAgICAgICAgLS1XZWJEYXRhU291cmNlQ291 bnQ7CiAKICNpZiBFTkFCTEUoRElTS19JTUFHRV9DQUNIRSkgJiYgUExBVEZPUk0oSU9TKQotICAg IGlmIChfcHJpdmF0ZSkgeworICAgIGlmIChfcHJpdmF0ZSAmJiBbc2VsZiBkYXRhU291cmNlRGVs ZWdhdGVdKSB7CiAgICAgICAgIFJlZlB0cjxSZXNvdXJjZUJ1ZmZlcj4gbWFpblJlc291cmNlQnVm ZmVyID0gdG9Qcml2YXRlKF9wcml2YXRlKS0+bG9hZGVyLT5tYWluUmVzb3VyY2VEYXRhKCk7CiAg ICAgICAgIGlmIChtYWluUmVzb3VyY2VCdWZmZXIpIHsKICAgICAgICAgICAgIFJlZlB0cjxTaGFy ZWRCdWZmZXI+IG1haW5SZXNvdXJjZURhdGEgPSBtYWluUmVzb3VyY2VCdWZmZXItPnNoYXJlZEJ1 ZmZlcigpOwo= 225284 2014-02-26 12:49:38 -0800 2014-02-27 11:06:21 -0800 Patch bug-129349-20140226124938.patch text/plain 2110 psolanki U3VidmVyc2lvbiBSZXZpc2lvbjogMTY0NzMzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L21h Yy9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0L21hYy9DaGFuZ2VMb2cKaW5kZXggNjhkYTljMjk4 MzA0YTQyMmFmMjlkZWRjMDU3ODg3ODkwNzdiYTU2ZS4uZTQyY2IyNmE2MTVmODlmNDYyYmQzYzE2 YTlkOWFkNDEyNzE4YzE3NSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9tYWMvQ2hhbmdlTG9n CisrKyBiL1NvdXJjZS9XZWJLaXQvbWFjL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE0 LTAyLTI2ICBQcmF0aWsgU29sYW5raSAgPHBzb2xhbmtpQGFwcGxlLmNvbT4KKworICAgICAgICBB c3NlcnRpb24gZmFpbHVyZSBhdCBDYWNoZWRSZXNvdXJjZS5oOjE5NjogQVNTRVJUKCFtX3B1cmdl YWJsZURhdGEpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xMjkzNDkKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzE0ODcxODM3PgorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBjb2RlIGZvciBjbGVhcmlu ZyBvdXQgbWVtb3J5IG1hcHBlZCBub3RpZmljYXRpb24gY2FsbGJhY2tzIGlzIG9ubHkgbmVlZGVk IHdoZW4gbG9hZGluZworICAgICAgICBQREZzLiBBbmQgaW4gc3VjaCBjYXNlcywgd2UgYWx3YXlz IGhhdmUgZGF0YVNvdXJjZURlbGVnYXRlIG9iamVjdC4gU28gbWFrZSB0aGlzIGNvZGUKKyAgICAg ICAgY29uZGl0aW9uYWwgb24gaXRzIHByZXNlbmNlIHNvIHRoYXQgd2UgZG9uJ3QgdHJpZ2dlciB0 aGUgYXNzZXJ0IGZvciBub24tUERGIG1haW4gcmVzb3VyY2VzLgorCisgICAgICAgICogV2ViVmll dy9XZWJEYXRhU291cmNlLm1tOgorICAgICAgICAoLVtXZWJEYXRhU291cmNlIGRlYWxsb2NdKToK KwogMjAxNC0wMi0yNSAgQW5kZXJzIENhcmxzc29uICA8YW5kZXJzY2FAYXBwbGUuY29tPgogCiAg ICAgICAgIEdldCByaWQgb2YgVmlzaXRlZExpbmtTdHJhdGVneQpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYktpdC9tYWMvV2ViVmlldy9XZWJEYXRhU291cmNlLm1tIGIvU291cmNlL1dlYktpdC9tYWMv V2ViVmlldy9XZWJEYXRhU291cmNlLm1tCmluZGV4IGE5NWZjNWVlYjM5MmI1MmQ4ZmFkZDAxMjIx Njc1YzE4YWIwOTU5ODAuLjQ0ZjE0NjRmOThiMzNhYjAyYmE5NzI5ODU3YjJlNGI1YzU1MTc0ZDYg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvbWFjL1dlYlZpZXcvV2ViRGF0YVNvdXJjZS5tbQor KysgYi9Tb3VyY2UvV2ViS2l0L21hYy9XZWJWaWV3L1dlYkRhdGFTb3VyY2UubW0KQEAgLTQ4MSw3 ICs0ODEsMTEgQEAgc3RhdGljIGlubGluZSB2b2lkIGFkZFR5cGVzRnJvbUNsYXNzKE5TTXV0YWJs ZURpY3Rpb25hcnkgKmFsbFR5cGVzLCBDbGFzcyBvYmpDQ2wKICAgICAgICAgLS1XZWJEYXRhU291 cmNlQ291bnQ7CiAKICNpZiBFTkFCTEUoRElTS19JTUFHRV9DQUNIRSkgJiYgUExBVEZPUk0oSU9T KQotICAgIGlmIChfcHJpdmF0ZSkgeworICAgIC8vIFRoZSBjb2RlIHRvIHJlbW92ZSBtZW1vcnkg bWFwcGVkIG5vdGlmaWNhdGlvbiBpcyBvbmx5IG5lZWRlZCB3aGVuIHdlIGFyZSB2aWV3aW5nIGEg UERGIGZpbGUuCisgICAgLy8gSW4gc3VjaCBhIGNhc2UsIFdlYlBERlZpZXdQbGFjZWhvbGRlciBz ZXRzIGl0c2VsZiBhcyB0aGUgZGF0YVNvdXJjZURlbGVnYXRlLiBHdWFyZCB0aGUgYWNjZXNzCisg ICAgLy8gdG8gbWFpblJlc291cmNlRGF0YSB3aXRoIHRoaXMgbmlsIGNoZWNrIHNvIHRoYXQgd2Ug YXZvaWQgYXNzZXJ0aW9ucyBkdWUgdG8gdGhlIHJlc291cmNlIGJlaW5nCisgICAgLy8gbWFkZSBw dXJnZWFibGUuCisgICAgaWYgKF9wcml2YXRlICYmIFtzZWxmIGRhdGFTb3VyY2VEZWxlZ2F0ZV0p IHsKICAgICAgICAgUmVmUHRyPFJlc291cmNlQnVmZmVyPiBtYWluUmVzb3VyY2VCdWZmZXIgPSB0 b1ByaXZhdGUoX3ByaXZhdGUpLT5sb2FkZXItPm1haW5SZXNvdXJjZURhdGEoKTsKICAgICAgICAg aWYgKG1haW5SZXNvdXJjZUJ1ZmZlcikgewogICAgICAgICAgICAgUmVmUHRyPFNoYXJlZEJ1ZmZl cj4gbWFpblJlc291cmNlRGF0YSA9IG1haW5SZXNvdXJjZUJ1ZmZlci0+c2hhcmVkQnVmZmVyKCk7 Cg==