[webkit-reviews] review granted: [Bug 48416] IDBFactoryBackend's reference to IDBDatabaseBackend should be weak : [Attachment 72011] patch

[bugzilla-daemon at webkit.org]

Steve Block <steveblock at google.com> has granted Jeremy Orlow
<jorlow at chromium.org>'s request for review:
Bug 48416: IDBFactoryBackend's reference to IDBDatabaseBackend should be weak
https://bugs.webkit.org/show_bug.cgi?id=48416

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

------- Additional Comments from Steve Block <steveblock at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=72011&action=review

r=me

>>> WebCore/storage/IDBDatabaseBackendImpl.h:93
>>> +	 RefPtr<IDBFactoryBackendImpl> m_factory;
>> 
>> You mention that the factory has the lifetime of the application. If so, why
do we need to take a ref to it?
> 
> The other objects are ref counted and held by javascript.  Are we guaranteed
that a GC will happen and the GC will catch _all_ references before the page
group goes away?
> 
> Is there any reason to do this either?  Given that the factory is a pretty
light weight object (since it only has weak references) there seems like no
advantage and big disadvantages (possible security bugs) for changing this
behavior.

OK, makes sense. I still think the comment could be more clear.