[webkit-reviews] review granted: [Bug 128117] Manage MediaQuery and MediaQueryExp classes through std::unique_ptr instead of OwnPtr : [Attachment 223011] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Zan Dobersek
<zandobersek at gmail.com>'s request for review:
Bug 128117: Manage MediaQuery and MediaQueryExp classes through std::unique_ptr
instead of OwnPtr
https://bugs.webkit.org/show_bug.cgi?id=128117

Attachment 223011: Patch
https://bugs.webkit.org/attachment.cgi?id=223011&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=223011&action=review


> Source/WebCore/css/CSSGrammar.y.in:522
> -	       $$ = MediaQueryExp::create(emptyString(), nullptr).leakPtr();
> +	       $$ = std::make_unique<MediaQueryExp>(emptyString(),
nullptr).release();

Elsewhere in this file we just use new for this sort of thing. Not sure how we
decide when to do make_unique/release vs. new.

> Source/WebCore/css/CSSGrammar.y.in:543
> +	   $$ = new Vector<std::unique_ptr<MediaQueryExp>>;

Here’s an example where we are using new (see comment above).

> Source/WebCore/css/MediaList.cpp:252
> +    const Vector<std::unique_ptr<MediaQuery>>& queries =
m_mediaQueries->queryVector();

I suggest "auto&" here instead of "const Vector<std::unique_ptr<MediaQuery>>&".


> Source/WebCore/css/MediaList.cpp:321
> +    const Vector<std::unique_ptr<MediaQuery>>& mediaQueries =
mediaQuerySet->queryVector();

I suggest "auto&" here instead of "const Vector<std::unique_ptr<MediaQuery>>&".


> Source/WebCore/css/MediaList.cpp:331
> +	       const Vector<std::unique_ptr<MediaQueryExp>>& expressions =
query->expressions();

I suggest "auto&" here instead of "const
Vector<std::unique_ptr<MediaQueryExp>>&".

> Source/WebCore/css/MediaQueryEvaluator.cpp:137
> +    const Vector<std::unique_ptr<MediaQuery>>& queries =
querySet->queryVector();

auto& would be better here

> Source/WebCore/css/MediaQueryEvaluator.cpp:150
> +	       const Vector<std::unique_ptr<MediaQueryExp>>& expressions =
query->expressions();

auto& would be better here