Bug 79421 - Need a WK1 Mac API to filter which subframes go into WebArchives as they are created
Summary: Need a WK1 Mac API to filter which subframes go into WebArchives as they are ...
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-02-23 16:49 PST by Brady Eidson
Modified: 2012-02-24 09:20 PST (History)
1 user (show)

See Also:


Attachments
Patch v1 (9.50 KB, patch)
2012-02-23 16:55 PST, Brady Eidson
sam: review-
Details | Formatted Diff | Diff
Patch v2 - Use a block (3.52 KB, patch)
2012-02-23 18:47 PST, Brady Eidson
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 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 Brady Eidson 2012-02-23 16:55:09 PST
Created attachment 128606 [details]
Patch v1
Comment 2 Sam Weinig 2012-02-23 17:51:13 PST
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.
Comment 3 Brady Eidson 2012-02-23 18:47:49 PST
Created attachment 128631 [details]
Patch v2 - Use a block
Comment 4 WebKit Commit Bot 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 Sam Weinig 2012-02-24 09:09:47 PST
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.
Comment 6 Adam Roben (:aroben) 2012-02-24 09:19:35 PST
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".)
Comment 7 Adam Roben (:aroben) 2012-02-24 09:20:12 PST
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.