[Webkit-unassigned] [Bug 22624] [SOUP][GTK] Need API to get SoupSession from WebKit.

[bugzilla-daemon at webkit.org]

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


christian at imendio.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #26741|review?                     |
               Flag|                            |
  Attachment #26741|0                           |1
        is obsolete|                            |
  Attachment #26836|                            |review?
               Flag|                            |




------- Comment #12 from christian at imendio.com  2009-01-18 14:43 PDT -------
Created an attachment (id=26836)
 --> (https://bugs.webkit.org/attachment.cgi?id=26836&action=view)
API to retrieve and set soup sessions #6

(In reply to comment #11)
> (In reply to comment #10)
> > Created an attachment (id=26741)
 --> (https://bugs.webkit.org/attachment.cgi?id=26741&action=view) [review] [review]
> > API to retrieve and set soup sessions #5
> +SoupSession* ResourceHandle::getSession(const Frame *frame)
> +{
> +    static SoupSession* default_session = soup_session_async_new();
> +
> +    if (!frame)
> +        return default_session;
> +    FrameLoader *loader = frame->loader();
> +    if (!loader)
> +        return 0;
> 
> Why not return the default session here? If this is some kind of catastrophic
> error situation maybe we should assert or something...

There is no catstrohpe, but I acknowledge you might think so if you don't see
it in the full context. In fact I defined getSession(0) to return default
session. Your question lead me to separate this into a function
getDefaultSession, to avoid confusion.

I think we must not assert actually, in case the view is being used while the
object is finalized. Nothing harmful should happen in any case.

> +    SoupSession* session =
> static_cast<FrameLoaderClient*>(loader->client())->session();
> +    return session ? session : default_session;
> +}
> 
> Also, just wondering in general, if the sessions are async maybe some checks
> wold be in order when swapping them (making sure they have no pending work?).
> Maybe that's related to your error...

My error came during browsing while keeping the default session, and while
using a non-default session, so it doesn't look like swapping is the problem. I
added an if(!session) to ensureSession now, and it looks like it prevents that
wanring. I'm still unsure however under what condition startHttp would have an
empty session...

> And as an optimization, I think a flag saying the session is initialized
> instead of checking for the features every time we go through startHttp would
> be good (I think I had this in previous patches).

I wonder about that, I was thinking you intended to make it dynamic later on.
Do you think the feature test is that slow? If we agree on which we prefer and
how to document that API wise, I can change it accordingly.


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