[webkit-reviews] review granted: [Bug 198206] Use a strongly-typed identifier for pages : [Attachment 370621] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun May 26 22:14:42 PDT 2019


youenn fablet <youennf at gmail.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 198206: Use a strongly-typed identifier for pages
https://bugs.webkit.org/show_bug.cgi?id=198206

Attachment 370621: Patch

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




--- Comment #34 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 370621
  --> https://bugs.webkit.org/attachment.cgi?id=370621
Patch

r=me once bots are happy.

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

> Source/WebCore/platform/network/CookieRequestHeaderFieldProxy.h:79
> +    result.pageID = WTFMove(*pageID);

No real need to move it.

> Source/WebCore/platform/network/NetworkStorageSession.h:153
> +    WEBCORE_EXPORT void grantStorageAccess(const RegistrableDomain&
resourceDomain, const RegistrableDomain& firstPartyDomain, Optional<uint64_t>
frameID, PageIdentifier);

Might be useful to add a specific FrameIdentifier type as well which I guess
would combine a PageIdentifier and a uint64_t.

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:-62
> -void CookieJarCurl::setCookiesFromDOM(const NetworkStorageSession& session,
const URL& firstParty, const SameSiteInfo&, const URL& url, Optional<uint64_t>
frameID, Optional<uint64_t> pageID, const String& value) const

We could remove pageID and remove UNUSED_PARAM(pageID) here and below.

> Source/WebKit/NetworkProcess/WebStorage/StorageManager.cpp:497
> +void StorageManager::createSessionStorageNamespace(PageIdentifier
storageNamespaceID, unsigned quotaInBytes)

A future refactoring should probably move away from PageIdentifier here.
If not really needed, I would probably not do these changes.


More information about the webkit-reviews mailing list