Bug 174641

Summary: [Curl] Move detail implementation from ResourceHandle to ResourceHandleInternal
Product: WebKit Reporter: Basuke Suzuki <Basuke.Suzuki>
Component: WebCore Misc.Assignee: Basuke Suzuki <Basuke.Suzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, Basuke.Suzuki, buildbot, commit-queue, galpeter
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 117300    
Attachments:
Description Flags
PATCH none

Description Basuke Suzuki 2017-07-18 12:40:16 PDT
When moving stuff from ResourceHandleManager to ResourceHandle on https://bugs.webkit.org/show_bug.cgi?id=173630, still there're many violation remains between ResourceHandle and ResourceHandleInternal classes. Many of implementation detail should be move to ResourceHandleInternal to improve build speed.
Comment 1 Basuke Suzuki 2017-07-18 13:04:20 PDT
Created attachment 315824 [details]
PATCH

Just moved to hide implementation from public.
Comment 2 Alex Christensen 2017-07-18 14:05:16 PDT
Comment on attachment 315824 [details]
PATCH

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

I don't think ResourceHandleInternal should've ever been separated from ResourceHandle.  They should've been one class.

Keep in mind, the end goal is to have all this networking code work with the NetworkDataTask abstraction.  Carlos Garcia did something similar with lib soup in these bugs, which were very heavily based on the asynchronous ResourceHandle work.
https://bugs.webkit.org/show_bug.cgi?id=163597
https://bugs.webkit.org/show_bug.cgi?id=163777
https://bugs.webkit.org/show_bug.cgi?id=163939

> Source/WebCore/platform/network/ResourceHandleInternal.h:92
> +        , m_handle { loader }

Ideally a ResourceHandleInternal wouldn't need to store a pointer to its ResourceHandle, but whatevs.
Comment 3 WebKit Commit Bot 2017-07-18 14:34:04 PDT
Comment on attachment 315824 [details]
PATCH

Clearing flags on attachment: 315824

Committed r219630: <http://trac.webkit.org/changeset/219630>
Comment 4 WebKit Commit Bot 2017-07-18 14:34:06 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Basuke Suzuki 2017-07-18 15:05:24 PDT
> I don't think ResourceHandleInternal should've ever been separated from
> ResourceHandle.  They should've been one class.

Again, please think this to improve the speed of further implementation. Build time it currently issue for productivity.


> Keep in mind, the end goal is to have all this networking code work with the
> NetworkDataTask abstraction.  Carlos Garcia did something similar with lib
> soup in these bugs, which were very heavily based on the asynchronous
> ResourceHandle work.
> https://bugs.webkit.org/show_bug.cgi?id=163597
> https://bugs.webkit.org/show_bug.cgi?id=163777
> https://bugs.webkit.org/show_bug.cgi?id=163939

I'll check them. 

> > Source/WebCore/platform/network/ResourceHandleInternal.h:92
> > +        , m_handle { loader }
> 
> Ideally a ResourceHandleInternal wouldn't need to store a pointer to its
> ResourceHandle, but whatevs.

Thanks for understanding. We remove that pretty soon.