[webkit-reviews] review granted: [Bug 208406] Batch observations and completions of text manipulations : [Attachment 392323] Fixed build

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Ryosuke Niwa
<rniwa at webkit.org>'s request for review:
Bug 208406: Batch observations and completions of text manipulations
https://bugs.webkit.org/show_bug.cgi?id=208406

Attachment 392323: Fixed build

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




--- Comment #4 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 392323
  --> https://bugs.webkit.org/attachment.cgi?id=392323
Fixed build

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

> Source/WebCore/editing/TextManipulationController.cpp:279
> +	   m_callback(*m_document, m_pendingItemsForCallback);
> +	   m_pendingItemsForCallback.clear();

Nit - you could call flushPendingItemsForCallback() here instead of repeating
the code.

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1658
> +	   // WebCore::TextManipulationController::ItemIdentifier itemID, const
Vector<WebCore::TextManipulationController::ManipulationToken>& tokens

Nit - stray comment?

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:1694
> +	       [wkItems addObject:[[_WKTextManipulationItem alloc]
initWithIdentifier:String::number(item.identifier.toUInt64())
tokens:wkTokens]];

This appears to be a leak — use RetainPtr/adoptNS or -autorelease?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:75
> +	   for (_WKTextManipulationItem* item in items)

Nit - * on the other side.