[webkit-reviews] review denied: [Bug 188033] [Curl] Crash on synchronous request via ResourceHandle. : [Attachment 345835] PATCH

[bugzilla-daemon at webkit.org]

Fujii Hironori <Hironori.Fujii at sony.com> has denied Basuke Suzuki
<Basuke.Suzuki at sony.com>'s request for review:
Bug 188033: [Curl] Crash on synchronous request via ResourceHandle.
https://bugs.webkit.org/show_bug.cgi?id=188033

Attachment 345835: PATCH

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




--- Comment #9 from Fujii Hironori <Hironori.Fujii at sony.com> ---
Comment on attachment 345835
  --> https://bugs.webkit.org/attachment.cgi?id=345835
PATCH

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

> LayoutTests/platform/wincairo/TestExpectations:863
> +http/tests/xmlhttprequest/simple-sync.html [ Pass ]

You added a new test case simple-sync.html in r234228.
https://trac.webkit.org/changeset/234228/
This is not a test gardening.
Test gardening is just marking test cases in TestExpectations.

In general, if you find a bug, create a test case and fixing the bug in a
single patch.
You don't need to split the commits.

> Source/WebCore/ChangeLog:8
> +	   The timing of instanciation of delegate was wrong. Move it inside

instanciation → instantiation

> Source/WebCore/ChangeLog:10
> +	   it's not null.

It's a bad idea to unify fixing bug and refactoring into a single patch.
Split this patch into two, fixing bug and refactoring.