151615 2015-11-25 23:24:20 -0800 [Content Filtering] Avoid creating a ContentFilter when loading the empty document 2015-11-30 10:06:35 -0800 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 aestes aestes ap beidson cdumez commit-queue dbates japhet kling oldest_to_newest 1144475 0 aestes 2015-11-25 23:24:20 -0800 [Content Filtering] Avoid creating a ContentFilter when loading the empty document 1144476 1 266174 aestes 2015-11-25 23:37:02 -0800 Created attachment 266174 Patch 1144539 2 266174 dbates 2015-11-26 11:19:26 -0800 Comment on attachment 266174 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=266174&action=review > Source/WebCore/loader/DocumentLoader.cpp:1421 > + m_contentFilter = !m_originalSubstituteDataWasValid ? ContentFilter::createIfEnabled(*this) : nullptr; I do not have easy access to search the repository at the moment. Can we write a test/do we have a test that verifies that we enable content filtering for the main resource when loaded from app cache? If not, then I suggest that we write one to ensure that we do not regress this behavior. You may be able to take inspiration from test <http://trac.webkit.org/browser/trunk/LayoutTests/http/tests/appcache/local-content.html> on how to load the main resource from app cache. 1144540 3 aestes 2015-11-26 12:11:13 -0800 (In reply to comment #2) > Comment on attachment 266174 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=266174&action=review > > > Source/WebCore/loader/DocumentLoader.cpp:1421 > > + m_contentFilter = !m_originalSubstituteDataWasValid ? ContentFilter::createIfEnabled(*this) : nullptr; > > I do not have easy access to search the repository at the moment. Can we > write a test/do we have a test that verifies that we enable content > filtering for the main resource when loaded from app cache? If not, then I > suggest that we write one to ensure that we do not regress this behavior. > You may be able to take inspiration from test > <http://trac.webkit.org/browser/trunk/LayoutTests/http/tests/appcache/local- > content.html> on how to load the main resource from app cache. This should be covered by http://trac.webkit.org/browser/trunk/LayoutTests/http/tests/contentfiltering/load-substitute-data-from-appcache.html. Thanks for the review! 1144795 4 aestes 2015-11-30 10:06:35 -0800 Committed r192797: <http://trac.webkit.org/changeset/192797> 266174 2015-11-25 23:37:02 -0800 2015-11-26 11:19:26 -0800 Patch bug-151615-20151125233645.patch text/plain 2531 aestes U3VidmVyc2lvbiBSZXZpc2lvbjogMTkyNzcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzM5M2RhN2U0ZDBlZDFh Mjk3MTMyNzUyODQzMmY5ZmRlNDk3OWM3ZS4uNjBiNTgxMDRhYTdhNDYyYmRjNjUwODI4MGJiZWU4 YWEzNGNjNjQxOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDE1LTExLTI1ICBBbmR5 IEVzdGVzICA8YWVzdGVzQGFwcGxlLmNvbT4KKworICAgICAgICBbQ29udGVudCBGaWx0ZXJpbmdd IEF2b2lkIGNyZWF0aW5nIGEgQ29udGVudEZpbHRlciB3aGVuIGxvYWRpbmcgdGhlIGVtcHR5IGRv Y3VtZW50CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0x NTE2MTUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJ dCdzIGV4cGVuc2l2ZSB0byBjcmVhdGUgdGhlIGZpcnN0IENvbnRlbnRGaWx0ZXIgc2luY2UgdHdv IGZyYW1ld29ya3MgbXVzdCBiZSBzb2Z0LWxpbmtlZC4gVGhlcmUncyBubyByZWFzb24gdG8KKyAg ICAgICAgcGF5IHRoaXMgY29zdCBpZiB3ZSdyZSBqdXN0IGxvYWRpbmcgdGhlIGVtcHR5IGRvY3Vt ZW50LgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gSXQncyBub3QgcG9zc2libGUgdG8gd3JpdGUg YSB0ZXN0IHRoYXQgd291bGQgZmFpbCB3aXRob3V0IHRoaXMgY2hhbmdlIHNpbmNlIENvbnRlbnRG aWx0ZXIgaXMgbm90CisgICAgICAgIG5vdGlmaWVkIG9mIGVtcHR5IGRvY3VtZW50IGxvYWRzLgor CisgICAgICAgICogbG9hZGVyL0RvY3VtZW50TG9hZGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkRvY3VtZW50TG9hZGVyOjpzdGFydExvYWRpbmdNYWluUmVzb3VyY2UpOiBEb24ndCBpbml0aWFs aXplIG1fY29udGVudEZpbHRlciB1bnRpbCB3ZSBrbm93IHdlIGFyZW4ndAorICAgICAgICBsb2Fk aW5nIHRoZSBlbXB0eSBkb2N1bWVudC4KKyAgICAgICAgKFdlYkNvcmU6OkRvY3VtZW50TG9hZGVy OjpEb2N1bWVudExvYWRlcik6CisKIDIwMTUtMTEtMTggIEFuZHkgRXN0ZXMgIDxhZXN0ZXNAYXBw bGUuY29tPgogCiAgICAgICAgIFtDb250ZW50IEZpbHRlcmluZ10gQ3Jhc2ggaW4gRG9jdW1lbnRM b2FkZXI6Om5vdGlmeUZpbmlzaGVkKCkgd2hlbiBhbGxvd2luZyBhIG1lZGlhIGRvY3VtZW50IHRv IGxvYWQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9Eb2N1bWVudExvYWRlci5j cHAgYi9Tb3VyY2UvV2ViQ29yZS9sb2FkZXIvRG9jdW1lbnRMb2FkZXIuY3BwCmluZGV4IGI4NGM5 NGNkZTg0MTE0NjM2NjJhMjY1YWU5YmMxNTg5Y2M3YzdlM2EuLmIyZWQ1ZGU2ZjZhZGYxZDcxMzJl ZDQ0ZmE1MmI5NDRkNDk5NjEyYmIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2xvYWRlci9E b2N1bWVudExvYWRlci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvbG9hZGVyL0RvY3VtZW50TG9h ZGVyLmNwcApAQCAtMTQ0LDkgKzE0NCw2IEBAIERvY3VtZW50TG9hZGVyOjpEb2N1bWVudExvYWRl cihjb25zdCBSZXNvdXJjZVJlcXVlc3QmIHJlcSwgY29uc3QgU3Vic3RpdHV0ZURhdGEmCiAgICAg LCBtX2RhdGFMb2FkVGltZXIoKnRoaXMsICZEb2N1bWVudExvYWRlcjo6aGFuZGxlU3Vic3RpdHV0 ZURhdGFMb2FkTm93KQogICAgICwgbV9zdWJyZXNvdXJjZUxvYWRlcnNBcmVQYWdlQ2FjaGVBY2Nl cHRhYmxlKGZhbHNlKQogICAgICwgbV9hcHBsaWNhdGlvbkNhY2hlSG9zdChzdGQ6Om1ha2VfdW5p cXVlPEFwcGxpY2F0aW9uQ2FjaGVIb3N0PigqdGhpcykpCi0jaWYgRU5BQkxFKENPTlRFTlRfRklM VEVSSU5HKQotICAgICwgbV9jb250ZW50RmlsdGVyKCFzdWJzdGl0dXRlRGF0YS5pc1ZhbGlkKCkg PyBDb250ZW50RmlsdGVyOjpjcmVhdGVJZkVuYWJsZWQoKnRoaXMpIDogbnVsbHB0cikKLSNlbmRp ZgogewogfQogCkBAIC0xNDIwLDYgKzE0MTcsMTAgQEAgdm9pZCBEb2N1bWVudExvYWRlcjo6c3Rh cnRMb2FkaW5nTWFpblJlc291cmNlKCkKICAgICBpZiAobWF5YmVMb2FkRW1wdHkoKSkKICAgICAg ICAgcmV0dXJuOwogCisjaWYgRU5BQkxFKENPTlRFTlRfRklMVEVSSU5HKQorICAgIG1fY29udGVu dEZpbHRlciA9ICFtX29yaWdpbmFsU3Vic3RpdHV0ZURhdGFXYXNWYWxpZCA/IENvbnRlbnRGaWx0 ZXI6OmNyZWF0ZUlmRW5hYmxlZCgqdGhpcykgOiBudWxscHRyOworI2VuZGlmCisKICAgICAvLyBG SVhNRTogSXMgdGhlcmUgYW55IHdheSB0aGUgZXh0cmEgZmllbGRzIGNvdWxkIGhhdmUgbm90IGJl ZW4gYWRkZWQgYnkgbm93PwogICAgIC8vIElmIG5vdCwgaXQgd291bGQgYmUgZ3JlYXQgdG8gcmVt b3ZlIHRoaXMgbGluZSBvZiBjb2RlLgogICAgIC8vIE5vdGUgdGhhdCBjdXJyZW50bHksIHNvbWUg cmVxdWVzdHMgbWF5IGhhdmUgaW5jb3JyZWN0IGV4dHJhIGZpZWxkcyBldmVuIGlmIHRoaXMgZnVu Y3Rpb24gaGFzIGJlZW4gY2FsbGVkLAo=