[webkit-reviews] review granted: [Bug 193622] [GTK][WPE] Add content extensions support in WKTR and unskip layout tests : [Attachment 360861] Patch v8

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Feb 2 14:58:27 PST 2019


Michael Catanzaro <mcatanzaro at igalia.com> has granted Adrian Perez
<aperez at igalia.com>'s request for review:
Bug 193622: [GTK][WPE] Add content extensions support in WKTR and unskip layout
tests
https://bugs.webkit.org/show_bug.cgi?id=193622

Attachment 360861: Patch v8

https://bugs.webkit.org/attachment.cgi?id=360861&action=review




--- Comment #36 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 360861
  --> https://bugs.webkit.org/attachment.cgi?id=360861
Patch v8

View in context: https://bugs.webkit.org/attachment.cgi?id=360861&action=review

This needs owner approval to land, so please find an owner to approve it.

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:241
> -	   processContentExtensionRulesForLoad(WTFMove(request), [this, handler
= WTFMove(handler), originalRequest = WTFMove(originalRequest)](auto result)
mutable {
> +	   this->processContentExtensionRulesForLoad(WTFMove(request), [this,
handler = WTFMove(handler), originalRequest = WTFMove(originalRequest)](auto
result) mutable {

You've already captured this. Why are you adding this here? Is it a bug in old
GCC?

> Source/WebKit/UIProcess/API/C/WKUserContentExtensionStoreRef.cpp:49
> +    RefPtr<API::ContentRuleListStore>
store(API::ContentRuleListStore::storeWithPath(toWTFString(path), false));

I've never understood the C/C++ APIs... do you really not need to adopt the
ref?

> Source/WebKit/UIProcess/API/C/WKUserContentExtensionStoreRef.h:48
> +
> +

Is the style really two blank lines?

> Tools/Scripts/webkitperl/FeatureList.pm:65
> +    $contentExtensionsSupport,
>      $cloopSupport,

It belongs under cloop.


More information about the webkit-reviews mailing list