Bug 211138

Summary: Make PolicyChecker an inner class of FrameLoader
Product: WebKit Reporter: Rob Buis <rbuis>
Component: New BugsAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, calvaris, cdumez, cgarcia, eric.carlson, ews-watchlist, glenn, gustavo, japhet, jer.noble, menard, philipj, pnormand, sergio, vjaquez, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Rob Buis
Reported 2020-04-28 11:59:51 PDT
PolicyChecker HistoryController an inner class of FrameLoader, this allows us to move some methods only used by PolicyChecker out of the FrameLoader public API. Because it is not possible to forward declare an enum class in an inner class, move ShouldContinue out of the PolicyChecker class and rename it to ShouldContinuePolicyCheck.
Attachments
Patch (41.64 KB, patch)
2020-04-28 12:03 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (41.59 KB, patch)
2020-04-28 13:58 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (41.32 KB, patch)
2020-04-28 14:00 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (41.04 KB, patch)
2020-04-29 01:22 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2020-04-28 12:03:19 PDT
Created attachment 397865 [details] Patch
Rob Buis
Comment 2 2020-04-28 13:58:35 PDT
Created attachment 397882 [details] Patch
Rob Buis
Comment 3 2020-04-28 14:00:22 PDT
Created attachment 397883 [details] Patch
Alex Christensen
Comment 4 2020-04-28 23:00:40 PDT
Comment on attachment 397883 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397883&action=review > Source/WebCore/loader/FrameLoader.h:112 > PolicyChecker& policyChecker() const { return *m_policyChecker; } You could make m_policyChecker a UniqueRef > Source/WebCore/loader/FrameLoaderTypes.h:115 > +enum class ShouldContinuePolicyCheck : uint8_t { bool > Source/WebCore/loader/PolicyChecker.h:114 > +template<> struct EnumTraits<WebCore::ShouldContinuePolicyCheck> { This is not needed if ShouldContinuePolicyCheck has a bool underlying type.
Rob Buis
Comment 5 2020-04-29 01:22:53 PDT
Created attachment 397940 [details] Patch
EWS
Comment 6 2020-04-29 03:23:06 PDT
Committed r260890: <https://trac.webkit.org/changeset/260890> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397940 [details].
Radar WebKit Bug Importer
Comment 7 2020-04-29 03:24:16 PDT
<rdar://problem/62587616>
Rob Buis
Comment 8 2020-04-29 06:09:30 PDT
Comment on attachment 397883 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397883&action=review >> Source/WebCore/loader/FrameLoader.h:112 >> PolicyChecker& policyChecker() const { return *m_policyChecker; } > > You could make m_policyChecker a UniqueRef Is there a clear benefit? I left it out for landing since there are a few similar cases in FrameLoader, I think it is better if needed to fix them all in one patch if there are benefits. >> Source/WebCore/loader/FrameLoaderTypes.h:115 >> +enum class ShouldContinuePolicyCheck : uint8_t { > > bool Done. >> Source/WebCore/loader/PolicyChecker.h:114 >> +template<> struct EnumTraits<WebCore::ShouldContinuePolicyCheck> { > > This is not needed if ShouldContinuePolicyCheck has a bool underlying type. Nice! I removed it.
Note You need to log in before you can comment on or make changes to this bug.