[webkit-reviews] review granted: [Bug 179190] [iOS-WK1] Fix thread safety issue in WebSQLiteDatabaseTrackerClient : [Attachment 325746] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 179190: [iOS-WK1] Fix thread safety issue in WebSQLiteDatabaseTrackerClient
https://bugs.webkit.org/show_bug.cgi?id=179190

Attachment 325746: Patch

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




--- Comment #5 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 325746
  --> https://bugs.webkit.org/attachment.cgi?id=325746
Patch

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

r=me if iOS Simulator layout test failure was not caused by this change.

> Source/WebCore/platform/ios/WebSQLiteDatabaseTrackerClient.mm:55
> +    ASSERT([NSThread isMainThread]);

Optionally (to avoid Objective-C overhead):

    ASSERT(ptrhread_main_np());

Not a blocker to land the patch.  Change at your discretion.

> Source/WebCore/platform/ios/WebSQLiteDatabaseTrackerClient.mm:65
> +    dispatch_async(dispatch_get_main_queue(), [this] {
> +	   ASSERT([NSThread isMainThread]);

This assertion seems redundant; if dispatch_async(dispatch_get_main_queue(),
...) doesn't work, we have bigger problems.  :)

> Source/WebCore/platform/ios/WebSQLiteDatabaseTrackerClient.mm:73
> +    dispatch_async(dispatch_get_main_queue(), [this] {
> +	   ASSERT([NSThread isMainThread]);

Ditto.

> Source/WebCore/platform/ios/WebSQLiteDatabaseTrackerClient.mm:80
> +    ASSERT([NSThread isMainThread]);

Optionally (to avoid Objective-C overhead):

    ASSERT(ptrhread_main_np());