[webkit-reviews] review granted: [Bug 95093] Update TestRunner API for web notifications : [Attachment 160734] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Jon Lee <jonlee at apple.com>'s
request for review:
Bug 95093: Update TestRunner API for web notifications
https://bugs.webkit.org/show_bug.cgi?id=95093

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

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=160734&action=review


I going to say r+, but I think that naming needs to be clarified.

> Tools/ChangeLog:22
> +	   (simulateLegacyWebNotificationClickCallback): Renamed.

Unsure if you are talking about "legacy web notifications", or a "legacy way to
simulate working with them". From the ChangeLog it sounds like the latter, but
function name shouts the former.

> Tools/DumpRenderTree/TestRunner.h:421
> -    bool m_areDesktopNotificationPermissionRequestsIgnored;
> +    bool m_areLegacyWebNotificationPermissionRequestsIgnored;

I could have used a comment here explaining what "legacy" means in this
context. Specifically, how will one determine that it's time to remove this
code?