[webkit-reviews] review granted: [Bug 184750] Add globally-unique HistoryItem identifiers (and have WebKit2 adopt them) : [Attachment 338309] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 184750: Add globally-unique HistoryItem identifiers (and have WebKit2 adopt
them)
https://bugs.webkit.org/show_bug.cgi?id=184750

Attachment 338309: Patch

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




--- Comment #3 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 338309
  --> https://bugs.webkit.org/attachment.cgi?id=338309
Patch

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

> Source/WebCore/history/BackForwardItemIdentifier.h:102
> +    static void constructDeletedValue(WebCore::BackForwardItemIdentifier&
slot) { slot.processIdentifier =
makeObjectIdentifier<WebCore::ProcessIdentifierType>(std::numeric_limits<uint64
_t>::max()); }

Please use
ObjectIdentifier<WebCore::ProcessIdentifierType>::hashTableDeletedValue().
Alternatively, define a constructor for HashTableDeletedValueType & add
isHashTableDeletedValue to BackForwardItemIdentifier?

> Source/WebKit/UIProcess/WebBackForwardList.cpp:422
> +	   backForwardListItemState.identifier = { Process::identifier(),
generateObjectIdentifier<BackForwardItemIdentifier::ItemIdentifierType>() };

Neat!

> Source/WebKit/WebProcess/WebPage/WebBackForwardListProxy.cpp:-158
> -    updateBackForwardItem(itemID, m_page->pageID(), item.ptr());

Why is it okay to stop calling updateBackForwardItem here? Could you clarify in
the change log?