[webkit-reviews] review requested: [Bug 40655] Database access in worker threads results in WebKit SPI notifications being posted from worker threads : [Attachment 71228] patch

[bugzilla-daemon at webkit.org]

Dumitru Daniliuc <dumi at chromium.org> has asked	for review:
Bug 40655: Database access in worker threads results in WebKit SPI
notifications being posted from worker threads
https://bugs.webkit.org/show_bug.cgi?id=40655

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

------- Additional Comments from Dumitru Daniliuc <dumi at chromium.org>
> > WebKit/mac/ChangeLog:5
> > +	     Repost the DatabaseTracker notifications to the main thread, if
needed.
> 
> This would probably be best as a function level comment. (And leave this area
for the bug title.)

to me, this looks like a pretty good description of the changes made to the
code in this dir... also, i think it's redundant to add the bug description to
the ChangeLog entry, since we already include a link to the bug. but i don't
feel strongly about this, so please let me know what you want me to put in the
"overall description" part of the ChangeLog entry (bug title?) and i'll do it.


> > WebKit/mac/ChangeLog:8
> > +	     Removing WebDatabaseTrackerClient::~WebDatabaseTrackerClient(),
> 
> If that is true, then leave the destructor declaration and remove the
implementation. Otherwise, you'll get the default destructor implementation.
> 
> (btw, this will likely generate the link error you had before, so you'll
likely need an implementation, but then this ChangeLog can be updated to be
more accurate.)
> 
> Also this type of comment would be better at the function level.

you're right, still getting those link errors. i added the destructors back.

> > WebKit/mac/Storage/WebDatabaseTrackerClient.mm:51
> > +class DidModifyOriginData {
> 
> Inherit from Noncopyable.

done.

> > WebKit/win/ChangeLog:8
> > +	     Removing WebDatabaseManager::~WebDatabaseManager(), because
> 
> Same comments as before.

did the same things i did for the changes in WebKit/mac.