[webkit-reviews] review granted: [Bug 195362] REGRESSION: (r242181) API test DragAndDropTests.ExternalSourcePlainTextToIFrame is Timing out : [Attachment 363976] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted youenn fablet
<youennf at gmail.com>'s request for review:
Bug 195362: REGRESSION: (r242181) API test
DragAndDropTests.ExternalSourcePlainTextToIFrame is Timing out
https://bugs.webkit.org/show_bug.cgi?id=195362

Attachment 363976: Patch

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




--- Comment #5 from Alexey Proskuryakov <ap at webkit.org> ---
Comment on attachment 363976
  --> https://bugs.webkit.org/attachment.cgi?id=363976
Patch

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

> Source/WebCore/page/SecurityOrigin.cpp:499
> +    ASSERT(!areOriginsMatching(origin1, origin2) || (origin1.toString() ==
origin2.toString()));

This assertion says that matching origins must have identical toString()
results. Why is this invariant important? This should be explained in a
comment, as otherwise someone will just remove it when it starts to fail after
another change.

For example, if toString() begins to output file URL host, which it kind of is
supposed to do, I'm not sure how it follows that we would have to treat the
URLs as not matching - in fact, file://localhost/ is usually same as file://.
This assertion alone does very little to explain why a change like the former
would be wrong.