[webkit-reviews] review granted: [Bug 207137] Crash in WebCore::IDBServer::IDBServer::createIndex : [Attachment 389581] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 207137: Crash in WebCore::IDBServer::IDBServer::createIndex
https://bugs.webkit.org/show_bug.cgi?id=207137

Attachment 389581: Patch

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




--- Comment #6 from Darin Adler <darin at apple.com> ---
Comment on attachment 389581
  --> https://bugs.webkit.org/attachment.cgi?id=389581
Patch

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

> Source/WebCore/ChangeLog:11
> +	   No behavior change.
> +
> +	   * Modules/indexeddb/shared/IDBIndexInfo.h:

This is not a great way to write the change log. Ideally the comment on the
file would say something like "Export IDBIndexInfo::isolatedCopy so it can be
used in WebKitLegacy framework code."

Replacing the line about testing with "No behavior change" is also not great.
Just deleting it is better. After all, the patch does create a behavior change,
although it’s the part of the change in WebKitLegacy rather than the part in
WebCore, so it’s not accurate.

> Source/WebKitLegacy/ChangeLog:9
> +	   Create an isolated copy of IDBIndexInfo before passing it to IDB
thread.

Seems like this comment should be on the function line rather than up here.