Bug 180545 - Web Inspector: Fix style in remote inspector classes
Summary: Web Inspector: Fix style in remote inspector classes
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
Keywords: DoNotImportToRadar
Depends on:
Reported: 2017-12-07 13:13 PST by Joseph Pecoraro
Modified: 2017-12-07 16:23 PST (History)
9 users (show)

See Also:

[PATCH] Proposed Fix (7.77 KB, patch)
2017-12-07 13:15 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2017-12-07 13:13:26 PST
Fix style in remote inspector classes

Make some things `const` and use `final` instead of `override` when possible.
Comment 1 Joseph Pecoraro 2017-12-07 13:15:16 PST
Created attachment 328726 [details]
[PATCH] Proposed Fix
Comment 2 youenn fablet 2017-12-07 13:18:41 PST
Comment on attachment 328726 [details]
[PATCH] Proposed Fix

Can you put some of the final methods as private instead of public?
Comment 3 Joseph Pecoraro 2017-12-07 15:59:48 PST
We probably can, but that rarely makes sense to me, especially if it was public in a parent class. I know I've seen this pattern elsewhere though, maybe someone can explain the advantage of that to me.
Comment 4 youenn fablet 2017-12-07 16:12:47 PST
We usually try to restrict the public methods to the minimum.
With this pattern, if B derives from A and C is the method being overridden, C can only be called in the context of code using A references/pointers.
There may be valid cases for which C can be called from B classes in which case B::C can be made public, but this does not seem to happen a lot.
Comment 5 WebKit Commit Bot 2017-12-07 16:22:59 PST
Comment on attachment 328726 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 328726

Committed r225654: <https://trac.webkit.org/changeset/225654>
Comment 6 WebKit Commit Bot 2017-12-07 16:23:00 PST
All reviewed patches have been landed.  Closing bug.