[Webkit-unassigned] [Bug 177934] [SOUP] Add support for preconnect

[bugzilla-daemon at webkit.org]

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

--- Comment #8 from Sergio Villar Senin <svillar at igalia.com> ---
(In reply to Sergio Villar Senin from comment #7)
> Comment on attachment 408225 [details]
> WIP patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=408225&action=review
> 
> > Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:247
> > +        soup_session_preconnect_async(soupSession, soupURI.get(), m_cancellable.get(),
> 
> This would require guarding with ENABLE(SERVER_PRECONNECT), actually the
> whole if block (otherwise weird things could happen).
> 
> I haven't checked the caller but I'll assume that
> parameters.shouldPreconnectOnly is properly guarded client side by
> ENABLE(SERVER_PRECONNECT) and could never be ::Yes if not enabled.
> 
> > Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:250
> > +                if (task.state() == State::Canceling || task.state() == State::Completed || !task.m_client)
> 
> Consider refactoring the condition to a method as it'll be used  in
> preconnectCallback too.
> 
> > Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp:341
> > +}
> 
> What about using scopes for the clearRequest()? Something like
> 
> auto clearRequestOnExit = makeScopeExit([task]() { task->clearRequest(); }
> 
> My only doubt is which is executed first the RefPtr::deref of protectedThis
> or the scope exit. Maybe it's safer to do it as you did then...

Sorry I was reviewing the old one. Forget about this.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210422/d00fcf60/attachment.htm>