[Webkit-unassigned] [Bug 74386] Migrate createObjectURL & revokeObjectURL to static (Class) methods

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=74386





--- Comment #41 from Dmitry Lomov <dslomov at google.com>  2012-02-01 09:28:23 PST ---
(From update of attachment 124918)
View in context: https://bugs.webkit.org/attachment.cgi?id=124918&action=review

Some lifetime commnents

> Source/WebCore/dom/ScriptExecutionContext.cpp:131
> +        m_publicURLManager->contextDestroyed();

If you end up not ref-counting PublicURLManager (see my comments below) maybe not have a separate PublicURLManager::contextDestroyed() method, and just do cleanup in PublicURLManager's destructor?

> Source/WebCore/dom/ScriptExecutionContext.cpp:392
> +PublicURLManager* ScriptExecutionContext::publicURLManager()

(see below comment on header file).

> Source/WebCore/dom/ScriptExecutionContext.h:110
> +    PublicURLManager* publicURLManager();

Since PublicURLManager is a RefCounted, this should return a RefPtr - never leak a naked pointer to ref-counted class.
Alternatively, I suggest *not* making PublicURLManager ref-counted - looks like it is only used in conjunction with owning ScriptExecutionContext and never stored anywhere else.

> Source/WebCore/dom/ScriptExecutionContext.h:215
> +    OwnPtr<PublicURLManager> m_publicURLManager;

Ditto, choose if ref-counted or not. If ref-counted, use RefPtr, not OwnPtr

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.