[webkit-reviews] review granted: [Bug 219949] Cannot see 'Add to my stations' button in BBC World service Radio : [Attachment 416522] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted
katherine_cheney at apple.com's request for review:
Bug 219949: Cannot see 'Add to my stations' button in BBC World service Radio
https://bugs.webkit.org/show_bug.cgi?id=219949

Attachment 416522: Patch

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




--- Comment #8 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 416522
  --> https://bugs.webkit.org/attachment.cgi?id=416522
Patch

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

> Source/WebCore/page/DOMWindow.cpp:2505
> +	   if (document->settings().needsSiteSpecificQuirks() &&
urlStringToOpen == Quirks::BBCRadioPlayerURLString()) {

It looks like you'll need to open Quirks.h to fix the windows build.

> Source/WebCore/page/Quirks.cpp:1155
> +	       if (auto subResourceDomainsInNeedOfStorageAccess =
NetworkStorageSession::subResourceDomainsInNeedOfStorageAccessForFirstParty(fir
stPartyDomain)) {

We tend to avoid deeply nested code like this by adding early returns.
auto subResourceDomainsInNeedOfStorageAccess = ;
if (!subResourceDomainsInNeedOfStorageAccess)
    return StorageAccessResult::ShouldNotCancelEvent;
if (storageAccessGranted != StorageAccessWasGranted::Yes)
    return StorageAccessResult::ShouldNotCancelEvent;
etc.

> Source/WebCore/page/Quirks.cpp:1168
> +				       if (abstractFrame &&
is<Frame>(*abstractFrame)) {

I think there's a version of is that takes a pointer and returns false if it's
null.