[webkit-reviews] review granted: [Bug 44865] REGRESSION (r66156): Sites using AppleConnect for authentication fail to log in. : [Attachment 65904] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Andy Estes <aestes at apple.com>'s
request for review:
Bug 44865: REGRESSION (r66156): Sites using AppleConnect for authentication
fail to log in.
https://bugs.webkit.org/show_bug.cgi?id=44865

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

------- Additional Comments from Darin Adler <darin at apple.com>
> +	   bool forceFallback = frame->settings()->needsSiteSpecificQuirks() &&
document()->url().host() == "wdg2.apple.com";

If you put this into an inline function rather than a boolean local variable
then you would not be evaluating it if beforeLoadAllowedLoad is true. In the
current code you always evaluate this, even if you don't use its result. I
think this needs a "why" comment in the code. It can be a brief one.

Is there a guarantee that frame->settings() can't be 0? What if we are in a
frame after it is no longer connected to a page? Then frame->settings() will be
0. Are we guaranteed safe from that?

Seems OK, r=me