Apply ContentExtension actions after redirects rdar://problem/20062613
Created attachment 249422 [details] WIP for posterity This is probably a pretty final-ish patch, but I'm working on layout tests to cover both main resources and subresources.
Created attachment 249425 [details] Patch v1
Attachment 249425 [details] did not pass style-queue: ERROR: Tools/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] ERROR: Source/WebCore/contentextensions/ContentExtensionsBackend.h:68: The parameter name "request" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 249425 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=249425&action=review > LayoutTests/http/tests/contentextensions/subresource-redirect-blocked.html:1 > +This page has an image.<br> Don't you need subresource-redirect-blocked-expected.txt? > Source/WebCore/contentextensions/ContentExtensionsBackend.h:68 > + void processContentExtensionRulesForLoad(ResourceRequest& request, ResourceType, DocumentLoader& initiatingDocumentLoader); request > Source/WebCore/loader/NetscapePlugInStreamLoader.cpp:48 > + m_resourceType = ResourceType::PlugInStream; This should be m_resourceType(ResourceType::PlugInStream) > Tools/ChangeLog:4 > + Need a short description (OOPS!). > + Need the bug URL (OOPS!). forgot
(In reply to comment #4) > Comment on attachment 249425 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=249425&action=review > > > LayoutTests/http/tests/contentextensions/subresource-redirect-blocked.html:1 > > +This page has an image.<br> > > Don't you need subresource-redirect-blocked-expected.txt? Yup, forgot to git-add it > > > Source/WebCore/contentextensions/ContentExtensionsBackend.h:68 > > + void processContentExtensionRulesForLoad(ResourceRequest& request, ResourceType, DocumentLoader& initiatingDocumentLoader); > > request Yup, gone. > > > Source/WebCore/loader/NetscapePlugInStreamLoader.cpp:48 > > + m_resourceType = ResourceType::PlugInStream; > > This should be m_resourceType(ResourceType::PlugInStream) Can't be - m_resourceType belongs to the base class, can't be included in subclass initialized lists > > > Tools/ChangeLog:4 > > + Need a short description (OOPS!). > > + Need the bug URL (OOPS!). > > forgot I sure did!
Created attachment 249431 [details] Patch v2
Comment on attachment 249431 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=249431&action=review Looks good except these small things: > Source/WebCore/loader/NetscapePlugInStreamLoader.cpp:48 > + m_resourceType = ResourceType::PlugInStream; m_resourceType(ResourceType::PlugInStream) > Source/WebCore/loader/ResourceLoadInfo.h:44 > + PlugInStream = 0x0100, You also need to change LoadType::FirstParty LoadType::ThirdParty so this doesn't overlap. Maybe these should be made one enum so that is more obvious.
Comment on attachment 249431 [details] Patch v2 Attachment 249431 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5576653375799296 New failing tests: svg/as-image/svg-image-with-svg-data-uri.html svg/as-image/svg-image-with-data-uri-from-canvas.html svg/as-image/svg-image-with-data-uri-reloading.html svg/as-image/svg-image-with-data-uri-background.html svg/as-image/svg-image-with-data-uri-use-data-uri.svg svg/as-image/img-zoom-svg-stylesheet.html svg/as-image/svg-image-with-data-uri-images-disabled.html
Created attachment 249434 [details] Archive of layout-test-results from ews106 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
(In reply to comment #8) > Comment on attachment 249431 [details] > Patch v2 > > Attachment 249431 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/5576653375799296 > > New failing tests: > svg/as-image/svg-image-with-svg-data-uri.html > svg/as-image/svg-image-with-data-uri-from-canvas.html > svg/as-image/svg-image-with-data-uri-reloading.html > svg/as-image/svg-image-with-data-uri-background.html > svg/as-image/svg-image-with-data-uri-use-data-uri.svg > svg/as-image/img-zoom-svg-stylesheet.html > svg/as-image/svg-image-with-data-uri-images-disabled.html I knew you didn't run all tests on all platforms! Please check these before landing.
Comment on attachment 249431 [details] Patch v2 Attachment 249431 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5283148732563456 New failing tests: svg/as-image/svg-image-with-svg-data-uri.html svg/as-image/svg-image-with-data-uri-from-canvas.html svg/as-image/svg-image-with-data-uri-reloading.html svg/as-image/svg-image-with-data-uri-background.html svg/as-image/svg-image-with-data-uri-use-data-uri.svg svg/as-image/img-zoom-svg-stylesheet.html svg/as-image/svg-image-with-data-uri-images-disabled.html
Created attachment 249436 [details] Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Will definitely look at tests before landing.
(In reply to comment #7) > Comment on attachment 249431 [details] > Patch v2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=249431&action=review > > Looks good except these small things: > > > Source/WebCore/loader/NetscapePlugInStreamLoader.cpp:48 > > + m_resourceType = ResourceType::PlugInStream; > > m_resourceType(ResourceType::PlugInStream) As mentioned above - That's initializer syntax, but this can't be an initializer, as the variable belongs to the super class. > > > Source/WebCore/loader/ResourceLoadInfo.h:44 > > + PlugInStream = 0x0100, > > You also need to change LoadType::FirstParty LoadType::ThirdParty so this > doesn't overlap. Maybe these should be made one enum so that is more > obvious. That was *not* obvious. Will change.
http://trac.webkit.org/changeset/182016
The layout test failures were a missing null check on UserContentController that has now been added.