[webkit-reviews] review granted: [Bug 108285] Crashed while ref'ing DatabaseContext in DatabaseManager::interruptAllDatabasesForContext() : [Attachment 185565] The fix.

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 108285: Crashed while ref'ing DatabaseContext in
DatabaseManager::interruptAllDatabasesForContext()
https://bugs.webkit.org/show_bug.cgi?id=108285

Attachment 185565: The fix.
https://bugs.webkit.org/attachment.cgi?id=185565&action=review

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


r=me on ThreadSafeRefCounted part.

> Source/WebCore/Modules/webdatabase/DatabaseManager.cpp:353
> -void
DatabaseManager::interruptAllDatabasesForContext(ScriptExecutionContext*
context)
> +void DatabaseManager::interruptAllDatabasesForContext(const
ScriptExecutionContext* context)

I do not think that we should be using "const ScriptExecutionContext*" here, or
anywhere.

These are huge "world" objects that are never actually immutable, and saying
that they are constant for the purposes of a particular function does not have
any semantic meaning that I could catch.

For example, you are passing context as constant here. But interrupting all
databases for context modifies the context in a very noticeable way!