135548 2014-08-03 00:20:52 -0700 QuickLook resources are cache-replaced with their original binary data causing ASSERT(m_data->size() == newBuffer->size()) in CachedResource.cpp 2014-08-04 10:19:41 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 psolanki psolanki aestes ap beidson commit-queue dbates ddkilzer japhet psolanki oldest_to_newest 1026507 0 psolanki 2014-08-03 00:20:52 -0700 When loading QuickLook resources, the SharedBuffer in the CachedResource is actually a converted representation of the real quicklook resource. Later, when we get the notification from Networking process to replace the data with the file backed buffer, the assets in tryReplaceEncodedData() get triggered. We should disable the replacement of data for quicklook resources. It's possible that we may show corrupted data if we later tried to display the quicklook binary data. I have not been able to trigger this though. <rdar://problem/17891321> 1026509 1 235942 psolanki 2014-08-03 00:34:30 -0700 Created attachment 235942 Patch 1026530 2 235942 ddkilzer 2014-08-03 13:07:07 -0700 Comment on attachment 235942 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235942&action=review r=me, but please removed the unused method disableEncodedDataReplacement() unless there is a missing source file for this patch! > Source/WebCore/loader/ResourceLoader.cpp:67 > +#if USE(QUICK_LOOK) > + , m_quickLookResource(false) > +#endif It would make the code a bit more readable if you removed this code from USE(QUICK_LOOK) and only set m_quickLookResource to true in ResourceLoader::didCreateQuickLookHandle(). > Source/WebCore/loader/ResourceLoader.h:227 > +#if USE(QUICK_LOOK) > +public: > + bool isQuickLookResource() { return m_quickLookResource; } > +private: > + bool m_quickLookResource; > +#endif Ditto. > Source/WebCore/loader/cache/CachedRawResource.cpp:45 > +#if USE(QUICK_LOOK) > + , m_allowEncodedDataReplacement(true) > +#endif It would make the code a bit more readable if you removed this code from USE(QUICK_LOOK) and only set m_allowEncodedDataReplacement to true in CachedRawResource::finishLoading(). > Source/WebCore/loader/cache/CachedRawResource.h:51 > +#if USE(QUICK_LOOK) > + void disableEncodedDataReplacement() { m_allowEncodedDataReplacement = false; } > +#endif This new method doesn't appear to be used anywhere? > Source/WebCore/loader/cache/CachedRawResource.h:71 > +#if USE(QUICK_LOOK) > + virtual bool mayTryReplaceEncodedData() const override { return m_allowEncodedDataReplacement; } > +#else > virtual bool mayTryReplaceEncodedData() const override { return true; } > +#endif It would make the code a bit more readable if you removed this code from USE(QUICK_LOOK) and only set m_allowEncodedDataReplacement to true in CachedRawResource::finishLoading(). > Source/WebCore/loader/cache/CachedRawResource.h:86 > +#if USE(QUICK_LOOK) > + bool m_allowEncodedDataReplacement; > +#endif Ditto. 1026535 3 235942 dbates 2014-08-03 14:42:39 -0700 Comment on attachment 235942 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235942&action=review > Source/WebCore/loader/ResourceLoader.cpp:66 > + , m_quickLookResource(false) We prefix the name of the getter function with "m_" as use that as the name of the underlying instance variable. So, this should be named m_isQuickLookResource. From what I recall this also makes the name of the instance variable consistent with the Cocoa naming conventions (which we tend to take inspiration from in WebKit) when naming boolean variables and getters/setters. 1026645 4 psolanki 2014-08-04 10:19:41 -0700 Committed r171993: <http://trac.webkit.org/changeset/171993> 235942 2014-08-03 00:34:30 -0700 2014-08-03 14:42:39 -0700 Patch bug-135548-20140803003417.patch text/plain 6181 psolanki U3VidmVyc2lvbiBSZXZpc2lvbjogMTcxOTcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZGY5MmJjMjRlY2U3Njg1 OWFmZDI5MDQyZGMwMTQ4YjUyNTc5NmVjYy4uODgyMWVjMjgyNmNjYTkwZGNjNGFhMTFiNTkzOTQ4 ODg0OThlNDBhZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDM2IEBACisyMDE0LTA4LTAzICBQcmF0 aWsgU29sYW5raSAgPHBzb2xhbmtpQGFwcGxlLmNvbT4KKworICAgICAgICBRdWlja0xvb2sgcmVz b3VyY2VzIGFyZSBjYWNoZS1yZXBsYWNlZCB3aXRoIHRoZWlyIG9yaWdpbmFsIGJpbmFyeSBkYXRh IGNhdXNpbmcgQVNTRVJUKG1fZGF0YS0+c2l6ZSgpID09IG5ld0J1ZmZlci0+c2l6ZSgpKSBpbiBD YWNoZWRSZXNvdXJjZS5jcHAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTEzNTU0OAorICAgICAgICA8cmRhcjovL3Byb2JsZW0vMTc4OTEzMjE+CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV2hlbiBsb2FkaW5n IFF1aWNrTG9vayByZXNvdXJjZXMsIHRoZSBTaGFyZWRCdWZmZXIgaW4gdGhlIENhY2hlZFJlc291 cmNlIGlzIGFjdHVhbGx5IGEKKyAgICAgICAgY29udmVydGVkIHJlcHJlc2VudGF0aW9uIG9mIHRo ZSByZWFsIFF1aWNrTG9vayByZXNvdXJjZS4gUmVwbGFjaW5nIHRoaXMgd2l0aCB0aGUgYWN0dWFs CisgICAgICAgIG5ldHdvcmsgcmVzb3VyY2UgKHdoaWNoIGlzIHdoYXQgdHJ5UmVwbGFjZUVuY29k ZWREYXRhKCkgdHJpZWQgdG8gZG8pIGlzIHdyb25nIGFuZCB0cmlnZ2VyZWQKKyAgICAgICAgYXNz ZXJ0cyBpbiB0aGUgY29kZS4KKworICAgICAgICBGaXggdGhpcyBieSBoYXZpbmcgQ2FjaGVkUmF3 UmVzb3VyY2U6Om1heVRyeVJlcGxhY2VFbmNvZGVkRGF0YSgpIHJldHVybiBmYWxzZSBpZiB3ZSBh cmUKKyAgICAgICAgbG9hZGluZyBhIFF1aWNrTG9vayByZXNvdXJjZS4KKworICAgICAgICBObyBu ZXcgdGVzdHMgYmVjYXVzZSB3ZSBkb24ndCBoYXZlIGEgd2F5IHRvIHRlc3QgUXVpY2tMb29rIGRv Y3VtZW50cy4KKworICAgICAgICAqIGxvYWRlci9SZXNvdXJjZUxvYWRlci5jcHA6CisgICAgICAg IChXZWJDb3JlOjpSZXNvdXJjZUxvYWRlcjo6UmVzb3VyY2VMb2FkZXIpOgorICAgICAgICAoV2Vi Q29yZTo6UmVzb3VyY2VMb2FkZXI6OmRpZENyZWF0ZVF1aWNrTG9va0hhbmRsZSk6CisgICAgICAg ICAgICBTZXQgYSBmbGFnIHRvIGluZGljYXRlIHRoYXQgd2UgYXJlIGxvYWRpbmcgYSBRdWlja0xv b2sgZG9jdW1lbnQuCisgICAgICAgICogbG9hZGVyL1Jlc291cmNlTG9hZGVyLmg6CisgICAgICAg IChXZWJDb3JlOjpSZXNvdXJjZUxvYWRlcjo6aXNRdWlja0xvb2tSZXNvdXJjZSk6CisgICAgICAg ICogbG9hZGVyL2NhY2hlL0NhY2hlZFJhd1Jlc291cmNlLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkNhY2hlZFJhd1Jlc291cmNlOjpDYWNoZWRSYXdSZXNvdXJjZSk6CisgICAgICAgIChXZWJDb3Jl OjpDYWNoZWRSYXdSZXNvdXJjZTo6ZmluaXNoTG9hZGluZyk6CisgICAgICAgICAgICBDaGVjayBp ZiB3ZSB3ZXJlIGxvYWRpbmcgYSBRdWlja0xvb2sgZG9jdW1lbnQgYW5kIGlmIHNvIGRpc2FibGUg ZW5jb2RlZCBkYXRhCisgICAgICAgICAgICByZXBsYWNlbWVudC4KKyAgICAgICAgKiBsb2FkZXIv Y2FjaGUvQ2FjaGVkUmF3UmVzb3VyY2UuaDoKKyAgICAgICAgICAgIEFkZCBhIG5ldyBib29sIGZp ZWxkIHJldHVybmVkIGJ5IG1heVRyeVJlcGxhY2VFbmNvZGVkRGF0YSgpLiBEZWZhdWx0IGlzIHRy dWUgYnV0IGl0IGlzCisgICAgICAgICAgICBzZXQgdG8gZmFsc2UgaW4gZmluaXNoTG9hZGluZygp IGlmIHdlIHdlcmUgbG9hZGluZyBRdWlja0xvb2sgZG9jdW1lbnQuCisKIDIwMTQtMDgtMDIgIEJl bmphbWluIFBvdWxhaW4gIDxicG91bGFpbkBhcHBsZS5jb20+CiAKICAgICAgICAgVXBkYXRlIHRo ZSBTZWFyY2hGaWVsZFJlc3VsdHNCdXR0b25FbGVtZW50IHNoYWRvdyBQc2V1ZG8gSWQgd2hlbiBI VE1MSW5wdXRFbGVtZW50J3MgbWF4UmVzdWx0cyBjaGFuZ2UKZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJDb3JlL2xvYWRlci9SZXNvdXJjZUxvYWRlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIv UmVzb3VyY2VMb2FkZXIuY3BwCmluZGV4IDkxMzBkNDhmMTQ2NjlhNzYyMGVhNmMzMzUzYTBjYzlm ZjYxMTYzODMuLmNjYjc3MDMyMTg4OGRjZmMzODM2MjY4OGY4MWRhMzJlNmYwZTY4ZGMgMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9SZXNvdXJjZUxvYWRlci5jcHAKKysrIGIvU291 cmNlL1dlYkNvcmUvbG9hZGVyL1Jlc291cmNlTG9hZGVyLmNwcApAQCAtNjIsNiArNjIsOSBAQCBS ZXNvdXJjZUxvYWRlcjo6UmVzb3VyY2VMb2FkZXIoRnJhbWUqIGZyYW1lLCBSZXNvdXJjZUxvYWRl ck9wdGlvbnMgb3B0aW9ucykKICAgICAsIG1fY2FuY2VsbGF0aW9uU3RhdHVzKE5vdENhbmNlbGxl ZCkKICAgICAsIG1fZGVmZXJzTG9hZGluZyhmcmFtZS0+cGFnZSgpLT5kZWZlcnNMb2FkaW5nKCkp CiAgICAgLCBtX29wdGlvbnMob3B0aW9ucykKKyNpZiBVU0UoUVVJQ0tfTE9PSykKKyAgICAsIG1f cXVpY2tMb29rUmVzb3VyY2UoZmFsc2UpCisjZW5kaWYKIHsKIH0KIApAQCAtNjE4LDYgKzYyMSw3 IEBAIHZvaWQgUmVzb3VyY2VMb2FkZXI6OnVuc2NoZWR1bGUoU2NoZWR1bGVQYWlyJiBwYWlyKQog I2lmIFVTRShRVUlDS19MT09LKQogdm9pZCBSZXNvdXJjZUxvYWRlcjo6ZGlkQ3JlYXRlUXVpY2tM b29rSGFuZGxlKFF1aWNrTG9va0hhbmRsZSYgaGFuZGxlKQogeworICAgIG1fcXVpY2tMb29rUmVz b3VyY2UgPSB0cnVlOwogICAgIGZyYW1lTG9hZGVyKCktPmNsaWVudCgpLmRpZENyZWF0ZVF1aWNr TG9va0hhbmRsZShoYW5kbGUpOwogfQogI2VuZGlmCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29y ZS9sb2FkZXIvUmVzb3VyY2VMb2FkZXIuaCBiL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9SZXNvdXJj ZUxvYWRlci5oCmluZGV4IDk3ZTM2NTFkMjc4MDc4NGMyMWRlZDg1NzgxMmZhNzg0ZWUxZmFlYzQu LmNjNTUxMTBkZWJjZWI3MDgzZjc4MmI5ODdjODQ0MmY3MmY3NjE4MDggMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJDb3JlL2xvYWRlci9SZXNvdXJjZUxvYWRlci5oCisrKyBiL1NvdXJjZS9XZWJDb3Jl L2xvYWRlci9SZXNvdXJjZUxvYWRlci5oCkBAIC0yMTksNiArMjE5LDEyIEBAIHByaXZhdGU6CiAg ICAgYm9vbCBtX2RlZmVyc0xvYWRpbmc7CiAgICAgUmVzb3VyY2VSZXF1ZXN0IG1fZGVmZXJyZWRS ZXF1ZXN0OwogICAgIFJlc291cmNlTG9hZGVyT3B0aW9ucyBtX29wdGlvbnM7CisjaWYgVVNFKFFV SUNLX0xPT0spCitwdWJsaWM6CisgICAgYm9vbCBpc1F1aWNrTG9va1Jlc291cmNlKCkgeyByZXR1 cm4gbV9xdWlja0xvb2tSZXNvdXJjZTsgfQorcHJpdmF0ZToKKyAgICBib29sIG1fcXVpY2tMb29r UmVzb3VyY2U7CisjZW5kaWYKIH07CiAKIGlubGluZSBjb25zdCBSZXNvdXJjZVJlc3BvbnNlJiBS ZXNvdXJjZUxvYWRlcjo6cmVzcG9uc2UoKSBjb25zdApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNv cmUvbG9hZGVyL2NhY2hlL0NhY2hlZFJhd1Jlc291cmNlLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2xv YWRlci9jYWNoZS9DYWNoZWRSYXdSZXNvdXJjZS5jcHAKaW5kZXggYjU1MmY3ZmI0ZDEwN2NhMDNl MzhlNTliMTUyYzNjZjA3OTAwOGYzNy4uOWY3OTUxMzM5OGMwYTM3MjFkYjVhNzBhZjM1YjBmOTEy NTYxNDhiYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvbG9hZGVyL2NhY2hlL0NhY2hlZFJh d1Jlc291cmNlLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvY2FjaGUvQ2FjaGVkUmF3 UmVzb3VyY2UuY3BwCkBAIC00MCw2ICs0MCw5IEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKIENhY2hl ZFJhd1Jlc291cmNlOjpDYWNoZWRSYXdSZXNvdXJjZShSZXNvdXJjZVJlcXVlc3QmIHJlc291cmNl UmVxdWVzdCwgVHlwZSB0eXBlLCBTZXNzaW9uSUQgc2Vzc2lvbklEKQogICAgIDogQ2FjaGVkUmVz b3VyY2UocmVzb3VyY2VSZXF1ZXN0LCB0eXBlLCBzZXNzaW9uSUQpCiAgICAgLCBtX2lkZW50aWZp ZXIoMCkKKyNpZiBVU0UoUVVJQ0tfTE9PSykKKyAgICAsIG1fYWxsb3dFbmNvZGVkRGF0YVJlcGxh Y2VtZW50KHRydWUpCisjZW5kaWYKIHsKICAgICAvLyBGSVhNRTogVGhlIHdyb25nIENhY2hlZFJl c291cmNlOjpUeXBlIGhlcmUgbWF5IGNhdXNlIGEgYmFkIGNhc3QgZWxzZXdoZXJlLgogICAgIEFT U0VSVChpc01haW5PclJhd1Jlc291cmNlKCkpOwpAQCAtOTUsNiArOTgsMTAgQEAgdm9pZCBDYWNo ZWRSYXdSZXNvdXJjZTo6ZmluaXNoTG9hZGluZyhSZXNvdXJjZUJ1ZmZlciogZGF0YSkKICAgICAg ICAgbm90aWZ5Q2xpZW50c0RhdGFXYXNSZWNlaXZlZChpbmNyZW1lbnRhbERhdGEsIGluY3JlbWVu dGFsRGF0YUxlbmd0aCk7CiAgICAgfQogCisjaWYgVVNFKFFVSUNLX0xPT0spCisgICAgbV9hbGxv d0VuY29kZWREYXRhUmVwbGFjZW1lbnQgPSAhbV9sb2FkZXItPmlzUXVpY2tMb29rUmVzb3VyY2Uo KTsKKyNlbmRpZgorCiAgICAgQ2FjaGVkUmVzb3VyY2U6OmZpbmlzaExvYWRpbmcoZGF0YSk7CiAg ICAgaWYgKGRhdGFCdWZmZXJpbmdQb2xpY3kgPT0gQnVmZmVyRGF0YSAmJiBtX29wdGlvbnMuZGF0 YUJ1ZmZlcmluZ1BvbGljeSgpID09IERvTm90QnVmZmVyRGF0YSkgewogICAgICAgICBpZiAobV9s b2FkZXIpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvY2FjaGUvQ2FjaGVkUmF3 UmVzb3VyY2UuaCBiL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9jYWNoZS9DYWNoZWRSYXdSZXNvdXJj ZS5oCmluZGV4IDNkNmJmYjI5NGVmN2M4MDVjYTc0NTg4YTA3YTY0NWI1ODdhMGY1OTUuLmJkNDA5 ODY2ZDFlYTk1M2E5MDZkZGZjYTQ0ZDg2NzIwY2IwOWM2MGYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJDb3JlL2xvYWRlci9jYWNoZS9DYWNoZWRSYXdSZXNvdXJjZS5oCisrKyBiL1NvdXJjZS9XZWJD b3JlL2xvYWRlci9jYWNoZS9DYWNoZWRSYXdSZXNvdXJjZS5oCkBAIC00Niw2ICs0NiwxMCBAQCBw dWJsaWM6CiAKICAgICB2b2lkIGNsZWFyKCk7CiAKKyNpZiBVU0UoUVVJQ0tfTE9PSykKKyAgICB2 b2lkIGRpc2FibGVFbmNvZGVkRGF0YVJlcGxhY2VtZW50KCkgeyBtX2FsbG93RW5jb2RlZERhdGFS ZXBsYWNlbWVudCA9IGZhbHNlOyB9CisjZW5kaWYKKwogcHJpdmF0ZToKICAgICB2aXJ0dWFsIHZv aWQgZGlkQWRkQ2xpZW50KENhY2hlZFJlc291cmNlQ2xpZW50Kikgb3ZlcnJpZGU7CiAgICAgdmly dHVhbCB2b2lkIGFkZERhdGFCdWZmZXIoUmVzb3VyY2VCdWZmZXIqKSBvdmVycmlkZTsKQEAgLTYw LDcgKzY0LDExIEBAIHByaXZhdGU6CiAgICAgdmlydHVhbCB2b2lkIGRpZFNlbmREYXRhKHVuc2ln bmVkIGxvbmcgbG9uZyBieXRlc1NlbnQsIHVuc2lnbmVkIGxvbmcgbG9uZyB0b3RhbEJ5dGVzVG9C ZVNlbnQpIG92ZXJyaWRlOwogCiAgICAgdmlydHVhbCB2b2lkIHN3aXRjaENsaWVudHNUb1JldmFs aWRhdGVkUmVzb3VyY2UoKSBvdmVycmlkZTsKKyNpZiBVU0UoUVVJQ0tfTE9PSykKKyAgICB2aXJ0 dWFsIGJvb2wgbWF5VHJ5UmVwbGFjZUVuY29kZWREYXRhKCkgY29uc3Qgb3ZlcnJpZGUgeyByZXR1 cm4gbV9hbGxvd0VuY29kZWREYXRhUmVwbGFjZW1lbnQ7IH0KKyNlbHNlCiAgICAgdmlydHVhbCBi b29sIG1heVRyeVJlcGxhY2VFbmNvZGVkRGF0YSgpIGNvbnN0IG92ZXJyaWRlIHsgcmV0dXJuIHRy dWU7IH0KKyNlbmRpZgogCiAgICAgdmlydHVhbCBib29sIGNhblJldXNlKGNvbnN0IFJlc291cmNl UmVxdWVzdCYpIGNvbnN0IG92ZXJyaWRlOwogCkBAIC03Myw2ICs4MSwxMCBAQCBwcml2YXRlOgog CiAgICAgdW5zaWduZWQgbG9uZyBtX2lkZW50aWZpZXI7CiAKKyNpZiBVU0UoUVVJQ0tfTE9PSykK KyAgICBib29sIG1fYWxsb3dFbmNvZGVkRGF0YVJlcGxhY2VtZW50OworI2VuZGlmCisKICAgICBz dHJ1Y3QgUmVkaXJlY3RQYWlyIHsKICAgICBwdWJsaWM6CiAgICAgICAgIGV4cGxpY2l0IFJlZGly ZWN0UGFpcihjb25zdCBSZXNvdXJjZVJlcXVlc3QmIHJlcXVlc3QsIGNvbnN0IFJlc291cmNlUmVz cG9uc2UmIHJlZGlyZWN0UmVzcG9uc2UpCg==