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

[bugzilla-daemon at webkit.org]

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

--- Comment #7 from Sergio Villar Senin <svillar at igalia.com> ---
Comment on attachment 408225
  --> https://bugs.webkit.org/attachment.cgi?id=408225
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...

-- 
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/5839bc75/attachment-0001.htm>