<rdar://66271491>
Created attachment 424307 [details] Patch
Created attachment 424364 [details] Patch
Created attachment 425166 [details] Patch
Created attachment 425199 [details] Patch
Comment on attachment 425199 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425199&action=review r=me > Source/WebCore/dom/ScriptExecutionContext.cpp:408 > + if (resultMessage.is8Bit()) > + return tryMakeString("Unhandled Promise Rejection: ", StringView { resultMessage.characters8(), length }, addEllipsis ? "..." : ""); > + return tryMakeString("Unhandled Promise Rejection: ", StringView { resultMessage.characters16(), length }, addEllipsis ? "..." : ""); We can write StringView(resultMessage).left(length) > Source/WebCore/dom/ScriptExecutionContext.cpp:420 > + errorMessage = "Unhandled Promise Rejection"; Let's use "Unhandled Promise Rejection"_s so that we can avoid allocation for string storage.
Created attachment 425276 [details] Patch
commit-queue failed to commit attachment 425276 [details] to WebKit repository. To retry, please set cq+ flag again.
Committed r275521: <https://commits.webkit.org/r275521> All reviewed patches have been landed. Closing bug and clearing flags on attachment 425276 [details].
Comment on attachment 425276 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425276&action=review > Source/WebCore/dom/ScriptExecutionContext.cpp:414 > + if (!errorMessage && resultMessage.length() > shortLength) > + errorMessage = tryMakeErrorString(shortLength); Is this really helpful? I think the other two things (maximum length 200, failure case when even that fails) are enough. The 10 character one seems overkill!
(In reply to Darin Adler from comment #9) > Comment on attachment 425276 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=425276&action=review > > > Source/WebCore/dom/ScriptExecutionContext.cpp:414 > > + if (!errorMessage && resultMessage.length() > shortLength) > > + errorMessage = tryMakeErrorString(shortLength); > > Is this really helpful? I think the other two things (maximum length 200, > failure case when even that fails) are enough. The 10 character one seems > overkill! That's a fair point, it's unlikely this path will ever be taken. I was mostly following what we do in JavaScriptCore::LiteralParser.
(In reply to Tadeu Zagallo from comment #10) > I was mostly following what we do in JavaScriptCore::LiteralParser. I haven’t looked, but it seems likely it is too much there, too. So tempting to over-engineer!