[webkit-reviews] review denied: [Bug 58115] Gather data on modal dialogs shown during unload events : [Attachment 88878] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has denied Sreeram Ramachandran
<sreeram at google.com>'s request for review:
Bug 58115: Gather data on modal dialogs shown during unload events
https://bugs.webkit.org/show_bug.cgi?id=58115

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

------- Additional Comments from Ryosuke Niwa <rniwa at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=88878&action=review

> Source/WebCore/ChangeLog:8
> +
> +	   No new tests. (OOPS!)

You need to explain what kind of you change you're making here. Also, you need
to replace "No new tests. (OOPS!)" by an explanation as to why you're not
adding new tests.

> Source/WebCore/page/Chrome.cpp:63
> +static inline void willRunModalDialog(const Frame* frame, const
ChromeClient::DialogType& dialogType, const ChromeClient* client)
> +{
> +    if (frame->loader()->pageDismissalEventBeingDispatched())
> +	   client->willRunModalDialogDuringPageDismissal(dialogType);
> +}
> +

I don't think we normally define a function like this at the top of a file. 
Instead, we put it right above where it first appears.	In this case, right
before Chrome::runJavaScriptAlert.

> Source/WebKit/chromium/ChangeLog:7
> +

You need to explain what kind of change you're making here.