Bug 151615

Summary: [Content Filtering] Avoid creating a ContentFilter when loading the empty document
Product: WebKit Reporter: Andy Estes <aestes>
Component: New BugsAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, beidson, cdumez, commit-queue, dbates, japhet, kling
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch dbates: review+

Andy Estes
Reported 2015-11-25 23:24:20 PST
[Content Filtering] Avoid creating a ContentFilter when loading the empty document
Attachments
Patch (2.47 KB, patch)
2015-11-25 23:37 PST, Andy Estes 		dbates: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Andy Estes
Comment 1 2015-11-25 23:37:02 PST
Created attachment 266174 [details] Patch
Daniel Bates
Comment 2 2015-11-26 11:19:26 PST
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.
Andy Estes
Comment 3 2015-11-26 12:11:13 PST
(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!
Andy Estes
Comment 4 2015-11-30 10:06:35 PST
Committed r192797: <http://trac.webkit.org/changeset/192797>
Note You need to log in before you can comment on or make changes to this bug.