Bug 151615 - [Content Filtering] Avoid creating a ContentFilter when loading the empty document
Summary: [Content Filtering] Avoid creating a ContentFilter when loading the empty doc...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andy Estes
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-25 23:24 PST by Andy Estes
Modified: 2015-11-30 10:06 PST (History)
7 users (show)

See Also:


Attachments
Patch (2.47 KB, patch)
2015-11-25 23:37 PST, Andy Estes
dbates: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2015-11-25 23:24:20 PST
[Content Filtering] Avoid creating a ContentFilter when loading the empty document
Comment 1 Andy Estes 2015-11-25 23:37:02 PST
Created attachment 266174 [details]
Patch
Comment 2 Daniel Bates 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.
Comment 3 Andy Estes 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!
Comment 4 Andy Estes 2015-11-30 10:06:35 PST
Committed r192797: <http://trac.webkit.org/changeset/192797>