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

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied 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 200601: Patch
https://bugs.webkit.org/attachment.cgi?id=200601&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=200601&action=review


> I had attached the test patch before with name Test.html and respective saved
file in mhtml Test.mhtml from chromium (one Image is not saved in Test.mhtml
file)). Isn't is enough ??
> Should I attach agin same Test.html again.
> Actually I am not very used to bugzilla (this is my first time), so plz
guide.

Manual test attached on bugzilla are irrelevant for a patch. You need to have
an automated test with your patch.

Typically tests are written as LayoutTest or API tests. I am unfamiliar with
the testing of PageSerializer, you will probably find how it is tested by
looking at the previous changes on that class.

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

This should be a reference to avoid the ref(). (or alternatively get the
pointer in the temporary).