Bug 174641 - [Curl] Move detail implementation from ResourceHandle to ResourceHandleInternal
Summary: [Curl] Move detail implementation from ResourceHandle to ResourceHandleInternal
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Basuke Suzuki
URL:
Keywords:
Depends on:
Blocks: 117300
  Show dependency treegraph
 
Reported: 2017-07-18 12:40 PDT by Basuke Suzuki
Modified: 2017-07-18 15:05 PDT (History)
5 users (show)

See Also:


Attachments
PATCH (39.35 KB, patch)
2017-07-18 13:04 PDT, Basuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.