[webkit-reviews] review denied: [Bug 60332] Move StoredCredentials from ThreadableLoader to ResourceHandle : [Attachment 92530] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied Mihai Parparita
<mihaip at chromium.org>'s request for review:
Bug 60332: Move StoredCredentials from ThreadableLoader to ResourceHandle
https://bugs.webkit.org/show_bug.cgi?id=60332

Attachment 92530: Patch
https://bugs.webkit.org/attachment.cgi?id=92530&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
> The trade-off is that now FrameLoader.h includes ResourceHandle.h 

This doesn't seem to be an improvement for build speed. ThreadableLoader.h only
includes three WTF headers, while ResourceHandle.h includes many WebCore ones.
So, every build will be slightly slower, and there is a larger chance to
trigger a near-world rebuild.

If FrameLoader only needs the enum, perhaps in can be moved to a separate file.


> Note that this does fix an actual bug

Can the bug fix be tested?

+	 ResourceHandler::loadResourceSynchronously is where the
+	 StoredCredentials credentials value is actually used, so it makes more

+	 sense for the enum definition to live next to it.

I tend to agree with this. Also, ResourceHandle is lower level than
ThreadableLoader, so including ThreadableLoader.h in it is not so great.