Bug 59437 – Moar strict OwnPtr fixes for WebKit/chromium

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

Moar strict OwnPtr fixes for WebKit/chromium

[Adam Barth]

Created attachment 91079 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 91079 [details]
Patch

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

> Source/WebKit/chromium/src/WebDataSourceImpl.cpp:43
> +OwnPtr<WebPluginLoadObserver> WebDataSourceImpl::m_nextPluginLoadObserver;

Won't this make an exit-time destructor?

[Adam Barth]

(In reply to comment #2)
> (From update of attachment 91079 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=91079&action=review
> 
> > Source/WebKit/chromium/src/WebDataSourceImpl.cpp:43
> > +OwnPtr<WebPluginLoadObserver> WebDataSourceImpl::m_nextPluginLoadObserver;
> 
> Won't this make an exit-time destructor?

exit-time destructors are allowed in Chromium.  I can change this not be one, if you like.

[Adam Barth]

Created attachment 91109 [details]
Patch

[Adam Barth]

Patch updated to avoid exit-time destructor.

[David Levin]

Comment on attachment 91109 [details]
Patch

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

Glad you got rid of the exit time destructor (as it bothered me too when I glanced at this before going off to bed).

Only one comment to consider.

> Source/WebKit/chromium/src/WebDevToolsAgentImpl.cpp:319
> +void WebDevToolsAgent::interruptAndDispatch(MessageDescriptor* rawDescriptor)

Yeah for decent variable names.

> Source/WebKit/chromium/src/WebDevToolsAgentImpl.cpp:342
> +    PageScriptDebugServer::interruptAndRun(new DebuggerTask(descriptor.release()));

Seems odd that there is no adopt of the "new DebuggerTask" here.

[Adam Barth]

> > Source/WebKit/chromium/src/WebDevToolsAgentImpl.cpp:342
> > +    PageScriptDebugServer::interruptAndRun(new DebuggerTask(descriptor.release()));
> 
> Seems odd that there is no adopt of the "new DebuggerTask" here.

Good question:

    static void interruptAndRun(PassOwnPtr<Task>);

Ah!  I've figured it out.  I haven't been removing the LOOSE_PASS_OWN_PTR define in PassOwnPtr.h!

[WebKit Commit Bot]

Comment on attachment 91109 [details]
Patch

Clearing flags on attachment: 91109

Committed r84933: <http://trac.webkit.org/changeset/84933>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.