Bug 79421 – Need a WK1 Mac API to filter which subframes go into WebArchives as they are created

• Component: WebKit API
• Status: ASSIGNED

[Brady Eidson]

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>

[Brady Eidson]

Created attachment 128606 [details]
Patch v1

[Sam Weinig]

Comment on attachment 128606 [details]
Patch v1

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.

[Brady Eidson]

Created attachment 128631 [details]
Patch v2 - Use a block

[WebKit Commit Bot]

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.

[Sam Weinig]

Comment on attachment 128631 [details]
Patch v2 - Use a block

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.

[Adam Roben (:aroben)]

Comment on attachment 128631 [details]
Patch v2 - Use a block

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".)

[Adam Roben (:aroben)]

Comment on attachment 128631 [details]
Patch v2 - Use a block

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.