[webkit-reviews] review granted: [Bug 128839] Add WKBackForwardListDidChangeNotification : [Attachment 224248] Patch

[bugzilla-daemon at webkit.org]

mitz at webkit.org <mitz at webkit.org> has granted Anders Carlsson
<andersca at apple.com>'s request for review:
Bug 128839: Add WKBackForwardListDidChangeNotification
https://bugs.webkit.org/show_bug.cgi?id=128839

Attachment 224248: Patch
https://bugs.webkit.org/attachment.cgi?id=224248&action=review

------- Additional Comments from mitz at webkit.org <mitz at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=224248&action=review


> Source/WebKit2/UIProcess/API/Cocoa/WKBackForwardList.mm:36
> +WK_EXPORT NSString * const WKBackForwardListDidChangeNotification =
@"WKBackForwardListDidChangeNotification";
> +WK_EXPORT NSString * const WKBackForwardListAddedItemKey =
@"WKBackForwardListAddedItemKey";
> +WK_EXPORT NSString * const WKBackForwardListRemovedItemsKey =
@"WKBackForwardListRemovedItemsKey";

There’s no need to use WK_EXPORT here.

> Source/WebKit2/UIProcess/Cocoa/NavigationState.mm:398
> +    if (addedItem) {
> +	   keys.append(WKBackForwardListAddedItemKey);
> +	   values.append(wrapper(*addedItem));
> +    }
> +
> +    if (!removedItems.isEmpty()) {
> +	   Vector<id> removed;
> +	   removed.reserveInitialCapacity(removedItems.size());
> +
> +	   for (const auto& removedItem : removedItems)
> +	       removed.uncheckedAppend(wrapper(*removedItem));
> +
> +	   keys.append(WKBackForwardListRemovedItemsKey);
> +	   values.append([NSArray arrayWithObjects:removed.data()
count:removed.size()]);
> +    }
> +
> +    auto userInfo = adoptNS([[NSDictionary alloc]
initWithObjects:values.data() forKeys:keys.data() count:keys.size()]);

Why take the risk that someone might append to one vector and not the other?
Just create an NSMutableDictionary and use setObject:forKey: or the equivalent
subscript syntax.