[webkit-reviews] review granted: [Bug 222590] Maybe-regression(STP121): window.open flakily returning null : [Attachment 423246] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 222590: Maybe-regression(STP121): window.open flakily returning null
https://bugs.webkit.org/show_bug.cgi?id=222590

Attachment 423246: Patch

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




--- Comment #9 from Darin Adler <darin at apple.com> ---
Comment on attachment 423246
  --> https://bugs.webkit.org/attachment.cgi?id=423246
Patch

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

> Source/WebKit/ChangeLog:15
> +	   DidCommitLoadForFrame IPC (as well as preceeding load-related
messages to maintain relative ordering).

typo: "preceding"

I understand the benefit of setting the flag. What is the risk of setting this
flag on DidCommitLoadForFrame?

> Tools/TestWebKitAPI/Tests/WebKit/ModalAlertsSPI.cpp:156
> +	   WKRetainPtr<WKContextRef> context =
adoptWK(WKContextCreateWithConfiguration(nullptr));

auto?

> Tools/TestWebKitAPI/Tests/WebKit/ModalAlertsSPI.cpp:165
> +	   WKRetainPtr<WKStringRef> htmlString =
Util::toWK("<script>open('about:blank', '_blank')</script>");

auto?

> Tools/TestWebKitAPI/Tests/WebKit/ModalAlertsSPI.cpp:166
> +	   auto blankURL = adoptWK(WKURLCreateWithUTF8CString("about:blank"));

Seems unused.