Bug 79421

Summary: Need a WK1 Mac API to filter which subframes go into WebArchives as they are created
Product: WebKit Reporter: Brady Eidson <beidson@apple.com>
Component: WebKit APIAssignee: Brady Eidson <beidson@apple.com>
Status: ASSIGNED    
Severity: Normal CC: commit-queue@webkit.org
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: All   
Attachments:
Description Flags
Patch v1
sam: review-
Patch v2 - Use a block sam: review+

Description From 2012-02-23 16:49:19 PST
Need a WK1 Mac API to filter which subframes go into WebArchives as they are created 

This API will use the WebCore code landed as part of https://bugs.webkit.org/show_bug.cgi?id=77766

In radar as <rdar://problem/10805709>
------- Comment #1 From 2012-02-23 16:55:09 PST -------
Created an attachment (id=128606) [details]
Patch v1
------- Comment #2 From 2012-02-23 17:51:13 PST -------
(From update of attachment 128606 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=128606&action=review

> Source/WebKit/mac/DOM/WebDOMOperationsPrivate.h:53
> +@interface DOMNode (WebDOMNodeOperationsPrivate)
> +- (WebArchive *)webArchiveWithWebArchiveCreationDelegate:(id)webArchiveCreationDelegate;
> +@end

This should take a block, not an arbitrary object.
------- Comment #3 From 2012-02-23 18:47:49 PST -------
Created an attachment (id=128631) [details]
Patch v2 - Use a block
------- Comment #4 From 2012-02-24 02:22:16 PST -------
Attachment 128631 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit/mac/ChangeLog', u'Source/Web..." exit_code: 1
Source/WebKit/mac/DOM/WebDOMOperationsPrivate.h:53:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #5 From 2012-02-24 09:09:47 PST -------
(From update of attachment 128631 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=128631&action=review

Feel free to add an API test as well.

> Source/WebKit/mac/DOM/WebDOMOperations.mm:89
> +class WebFrameFilter : public WebCore::FrameFilter {
> +public:
> +    WebFrameFilter(ShouldIncludeSubframeInWebArchiveBlock filterBlock);
> +        
> +private:
> +    virtual bool shouldIncludeSubframe(Frame*) const OVERRIDE;
> +
> +    ShouldIncludeSubframeInWebArchiveBlock m_filterBlock;
> +};

Though perhaps not strictly necessary due to the one place it is used, it seems safer to Block_copy/Block_release this in the c++ class.
------- Comment #6 From 2012-02-24 09:19:35 PST -------
(From update of attachment 128631 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=128631&action=review

> Source/WebKit/mac/DOM/WebDOMOperationsPrivate.h:54
> +- (WebArchive *)webArchiveFilteringSubframes:(ShouldIncludeSubframeInWebArchiveBlock)shouldIncludeSubframeInWebArchive;

Maybe webArchiveWithSubframeFilter: would be a better name? And the block typedef could be WebArchiveSubframeFilter. (I'm not sure the block typedef needs to include the word "block".)
------- Comment #7 From 2012-02-24 09:20:12 PST -------
(From update of attachment 128631 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=128631&action=review

>> Source/WebKit/mac/DOM/WebDOMOperationsPrivate.h:54
>> +- (WebArchive *)webArchiveFilteringSubframes:(ShouldIncludeSubframeInWebArchiveBlock)shouldIncludeSubframeInWebArchive;
> 
> Maybe webArchiveWithSubframeFilter: would be a better name? And the block typedef could be WebArchiveSubframeFilter. (I'm not sure the block typedef needs to include the word "block".)

After all, this block is just being used to perform a functional-style filter operation.