[webkit-reviews] review granted: [Bug 115513] Improving PageSerializer.cpp to retrieve the subresources specified in @media rules in css : [Attachment 200668] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Santosh Mahto
<santoshbit2007 at gmail.com>'s request for review:
Bug 115513: Improving PageSerializer.cpp to retrieve the subresources specified
in @media rules in css
https://bugs.webkit.org/show_bug.cgi?id=115513

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

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


> Source/WebCore/page/PageSerializer.cpp:280
> +	       StyleRuleMedia* mediaStyle =
static_cast<StyleRuleMedia*>(styleSheet->contents()->ruleAt(i));

I think this might read better without this local variable.

> Source/WebCore/page/PageSerializer.cpp:283
> +	       size_t ruleSize = rules.size();

I think “rule size” is a strange name for this, since it’s not the size of a
rule. I suggest either rulesSize or just size for this local.

> Source/WebCore/page/PageSerializer.cpp:285
> +		   const RefPtr<StyleRuleBase>& rule = rules[i];

This is OK, but this is better:

    StyleRuleBase* rule = rules[i].get();

Since we have to do the get() on the next line anyway, it’s better to do it
right away to avoid const RefPtr&.