[Webkit-unassigned] [Bug 163597] [SOUP] Add NetworkSession implementation and switch to use it

[bugzilla-daemon at webkit.org]

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

Alex Christensen <achristensen at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #292081|review?                     |review+
              Flags|                            |

--- Comment #16 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 292081
  --> https://bugs.webkit.org/attachment.cgi?id=292081
Try to fix the builds

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

r=me

> Source/WebKit2/NetworkProcess/Downloads/Download.h:50
> +#if USE(NETWORK_SESSION)

Let's just put this into the #if USE(NETWORK_SESSION) right above it.

> Source/WebKit2/NetworkProcess/Downloads/Download.h:55
> +class NetworkDataTask;

Let's put this below.  Forward declaring an unused class doesn't cause problems.

> Source/WebKit2/NetworkProcess/Downloads/Download.h:59
> +#else
> +typedef void* PlatformDownloadTaskRef;

Let's not start defining things for a hypothetical third implementation.  If someone wants to make another implementation, they can define what they want.

> Source/WebKit2/NetworkProcess/Downloads/Download.h:148
>  #else
> +    PlatformDownloadTaskRef m_download;

Let's remove this.  It will need to be some kind of smart pointer anyways.

> Source/WebKit2/NetworkProcess/NetworkLoad.cpp:274
> +    if (m_task && m_task->isDownload())

isPendingDownload

> Source/WebKit2/NetworkProcess/soup/NetworkDataTaskSoup.cpp:52
> +    , m_session(&session)

It looks like we could make m_session a Ref.

> Source/WebKit2/NetworkProcess/soup/NetworkDataTaskSoup.cpp:144
> +    url.setQuery(String());
> +    url.removeFragmentIdentifier();

I think these are unnecessary

> Source/WebKit2/NetworkProcess/soup/NetworkSessionSoup.cpp:37
> +Ref<NetworkSession> NetworkSession::create(Type type, SessionID sessionID, CustomProtocolManager*)

Maybe in a followup patch, and definitely an existing problem from my implementation, but it seems like type and sessionID contain redundant information.  Type could be removed.

> Source/WebKit2/config.h:78
> -#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 100000)
> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 100000) || USE(SOUP)

I haven't tested this on linux.  If you want, you could land this as a separate change so if it needs to be reverted then it's easier to revert a one line change than all this code.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20161019/01189dfe/attachment-0001.html>