Bug 106012 – [SOUP] Convert WebSoupRequestManagerProxy to be WebContextSupplement

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Carlos Garcia Campos]

ssia

[Carlos Garcia Campos]

Created attachment 181167 [details]
Patch

[WebKit Review Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Martin Robinson]

Comment on attachment 181167 [details]
Patch

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

> Source/WebKit2/ChangeLog:7
> +        https://bugs.webkit.org/show_bug.cgi?id=106012
> +
> +        Reviewed by NOBODY (OOPS!).
> +

Could use a bit of context here or in the bug about why it makes sense to switch to this to a WebContextSupplement, for posterity.

> Source/WebKit2/UIProcess/soup/WebSoupRequestManagerProxy.cpp:98
> +    if (!context())
> +        return;

What's the mechanism by which the context becomes NULL. It's passed in as an argument, so does it become invalid at some point during the lifetime of the object?

[Carlos Garcia Campos]

(In reply to comment #3)
> (From update of attachment 181167 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181167&action=review
> 
> > Source/WebKit2/ChangeLog:7
> > +        https://bugs.webkit.org/show_bug.cgi?id=106012
> > +
> > +        Reviewed by NOBODY (OOPS!).
> > +
> 
> Could use a bit of context here or in the bug about why it makes sense to switch to this to a WebContextSupplement, for posterity.

To make the WebContext code more simple and cleaner, same reaoning as bug #105890, for example.

> > Source/WebKit2/UIProcess/soup/WebSoupRequestManagerProxy.cpp:98
> > +    if (!context())
> > +        return;
> 
> What's the mechanism by which the context becomes NULL. 

WebContextSupplement::clearContext()

> It's passed in as an argument, so does it become invalid at some point during the lifetime of the object?

It's cleared in the WebContext destructor, some supplements have the checks and some other don't, so I decided to include to be extra sure.

[Eric Seidel (no email)]

Attachment 181167 [details] was posted by a committer and has review+, assigning to Carlos Garcia Campos for commit.

[Carlos Garcia Campos]

Committed r138783: <http://trac.webkit.org/changeset/138783>