Summary: | Drop legacy WebCore::toRegistrableDomain() utility function | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | achristensen, beidson, commit-queue, dbates, esprehn+autocc, ews-watchlist, ggaren, japhet, kangil.han, webkit-bug-importer, wilander, youennf | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 195634 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Chris Dumez
2019-03-12 12:50:35 PDT
Created attachment 364436 [details]
Patch
Needs https://bugs.webkit.org/show_bug.cgi?id=195634 to land first. Comment on attachment 364436 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364436&action=review > Source/WebCore/dom/Document.cpp:3453 > + if (targetDocument && (targetDocument->securityOrigin().canAccess(SecurityOrigin::create(destinationURL)) || RegistrableDomain { targetDocument->url() } == RegistrableDomain { destinationURL })) Is there a reason why you prefer creating two objects over comparing one with the matches() function? > Source/WebCore/loader/CrossOriginAccessControl.cpp:233 > + if (RegistrableDomain { response.url() } != RegistrableDomain::uncheckedCreateFromHost(origin.host())) Ditto. You have the response URL. > Source/WebCore/loader/FrameLoader.cpp:1112 > + if (SecurityPolicy::shouldInheritSecurityOriginFromOwner(frame->document()->url()) || RegistrableDomain { frame->document()->url() } == registrableDomain) Ditto. > Source/WebCore/loader/FrameLoader.cpp:2973 > + request.setIsSameSite(RegistrableDomain { initiator->siteForCookies() } == RegistrableDomain { request.url() }); Ditto. > Source/WebCore/platform/network/ResourceRequestBase.h:-260 > -// FIXME: Find a better place for these functions. Yay, a FIXME gets fixed. > Source/WebCore/platform/network/cf/ResourceRequestCFNet.cpp:289 > + m_sameSiteDisposition = !siteForCookies ? SameSiteDisposition::Unspecified : (RegistrableDomain { siteForCookies.get() } == RegistrableDomain { m_url }) ? SameSiteDisposition::SameSite : SameSiteDisposition::CrossSite); Here you could also use the faster matches() function. > Source/WebCore/platform/network/cocoa/ResourceRequestCocoa.mm:92 > + m_sameSiteDisposition = siteForCookies.isNull() ? SameSiteDisposition::Unspecified : ((RegistrableDomain { siteForCookies } == RegistrableDomain { m_url }) ? SameSiteDisposition::SameSite : SameSiteDisposition::CrossSite); Ditto. > Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:148 > + return WebCore::RegistrableDomain { request.url() } != request.firstPartyForCookies(); If request.firstPartyForCookies() returns a RegistrableDomain, you could use matches() here too. > Source/WebKit/UIProcess/WebProcessPool.cpp:1312 > + updatedRequest.setIsSameSite(RegistrableDomain { initiatingPageURL } == RegistrableDomain { request.url() }); Can use matches(). > Source/WebKit/UIProcess/WebProcessPool.cpp:2299 > + if (!sourceURL.isValid() || !targetURL.isValid() || sourceURL.isEmpty() || sourceURL.protocolIsAbout() || RegistrableDomain { sourceURL } == targetRegistrableDomain) Ditto. > Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:398 > + if (m_hasFrameSpecificStorageAccess && RegistrableDomain { currentUrl } != RegistrableDomain { newUrl }) { Ditto. (In reply to John Wilander from comment #3) > Comment on attachment 364436 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=364436&action=review > > > Source/WebCore/dom/Document.cpp:3453 > > + if (targetDocument && (targetDocument->securityOrigin().canAccess(SecurityOrigin::create(destinationURL)) || RegistrableDomain { targetDocument->url() } == RegistrableDomain { destinationURL })) > > Is there a reason why you prefer creating two objects over comparing one > with the matches() function? I merely did not know about it. I'll update the patch, thanks for looking. Created attachment 364447 [details]
Patch
Created attachment 364549 [details]
Patch
Comment on attachment 364549 [details]
Patch
r=me
Comment on attachment 364549 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=364549&action=review > Source/WebCore/platform/RegistrableDomain.h:134 > +inline bool registrableDomainsAreEqual(const URL& a, const URL& b) Nit: I would name this areRegistrableDomainsEqual. I think seeing is/are as a prefix helps show that what follows is a boolean condition, and I think that's how Apple API is typically written. Created attachment 364554 [details]
Patch
Created attachment 364559 [details]
Patch
Comment on attachment 364559 [details] Patch Clearing flags on attachment: 364559 Committed r242899: <https://trac.webkit.org/changeset/242899> All reviewed patches have been landed. Closing bug. |