[webkit-reviews] review granted: [Bug 236844] Add assertion that no two sessions share the same general storage directory : [Attachment 453443] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 236844: Add assertion that no two sessions share the same general storage
directory
https://bugs.webkit.org/show_bug.cgi?id=236844

Attachment 453443: Patch

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




--- Comment #9 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 453443
  --> https://bugs.webkit.org/attachment.cgi?id=453443
Patch

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

> Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:211
> +    HashMap<String, PAL::SessionID> directoriesBySession;

This name seems wrong. If something says "BySession", then I'd expect the
session to be the HashMap key, not the value.
Maybe this could be named something like: "directorySessions" or
"sessionForDirectory"

> Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:216
> +		  
RELEASE_ASSERT_WITH_MESSAGE(directoriesBySession.add(directory,
sessionID).isNewEntry, "GeneralStorageDirectory for session '%llu' is already
in use by session '%llu'", directoriesBySession.get(directory).toUInt64(),
sessionID.toUInt64());

We're supposed to use PRIu64, not %llu to print 64bit integers to be friendly
to Linux ports. Looks like this code may not build on Linux but I think it I
still good practice.