[webkit-reviews] review denied: [Bug 42292] Add NetworkingContext to avoid layer violations : [Attachment 61977] NetworkingContext v1

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Jesus Sanchez-Palencia
<jesus at webkit.org>'s request for review:
Bug 42292: Add NetworkingContext to avoid layer violations
https://bugs.webkit.org/show_bug.cgi?id=42292

Attachment 61977: NetworkingContext v1
https://bugs.webkit.org/attachment.cgi?id=61977&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
I don’t think we necessarily need these two layers, NetworkingContext and
PlatformNextworkingContext. I’d want a pattern more like ResourceRequest, where
the Qt version of NetworkingContext has a pointer to a QWebFrame, but there’s
no ifdef in the main NetworkingContext header. Or maybe there’s another example
we can match. Even if we do want it all in one header, I think the
PlatformNetworkingContext thing is no good. Just have a QWebFrame* there for Qt
only. Some platforms might have more data inside a NetworkingContext than a
single pointer, and some might not need the pointer, so lets not force it on
everyone.

The networking context on FrameLoaderClientQt should be an OwnPtr, not a raw
pointer.

The initial patch should include replacing the existing Frame* argument in
ResourceHandle with a NetworkingContext*.

What's the lifetime of the NetworkingContext object? I don't think this is
expressed in a workable fashion in the current patch. It can be a reference
counted object, or one that has some other explicit way of maintaining its
lifetime. By adding the object but not uses of it, it allows us to have the
lifetime issue not yet solved, which I think is a problem.