Bug 155021 - Added missing override specifiers under Source/WebCore.
Summary: Added missing override specifiers under Source/WebCore.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-04 08:55 PST by Konstantin Tokarev
Modified: 2016-03-04 15:28 PST (History)
3 users (show)

See Also:


Attachments
Patch (24.31 KB, patch)
2016-03-04 08:59 PST, Konstantin Tokarev
no flags Details | Formatted Diff | Diff
Patch (23.14 KB, patch)
2016-03-04 10:23 PST, Konstantin Tokarev
no flags Details | Formatted Diff | Diff
Patch (23.10 KB, patch)
2016-03-04 13:26 PST, Konstantin Tokarev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Tokarev 2016-03-04 08:55:01 PST
Added missing override specifiers under Source/WebCore.
Comment 1 Konstantin Tokarev 2016-03-04 08:58:38 PST
Also added #if(SOUP) guard to ResourceHandleStreamingClient::getOrCreateReadBuffer because it is not an overridden method otherwise.
Comment 2 Konstantin Tokarev 2016-03-04 08:59:08 PST
Created attachment 272981 [details]
Patch
Comment 3 Darin Adler 2016-03-04 09:06:59 PST
Comment on attachment 272981 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=272981&action=review

Neat!

> Source/WebCore/page/DebugPageOverlays.cpp:62
> +    void pageOverlayDestroyed(PageOverlay&) final;
> +    void willMoveToPage(PageOverlay&, Page*) final;
> +    void didMoveToPage(PageOverlay&, Page*) final;
> +    void drawRect(PageOverlay&, GraphicsContext&, const IntRect& dirtyRect) final;
> +    bool mouseEvent(PageOverlay&, const PlatformMouseEvent&) final;
> +    void didScrollFrame(PageOverlay&, Frame&) final;

Looks wrong. We removed virtual here but did not add override. Maybe a bug in the tool.
Comment 4 WebKit Commit Bot 2016-03-04 10:00:16 PST
Comment on attachment 272981 [details]
Patch

Rejecting attachment 272981 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 272981, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
    -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 197565 = 1a4d6f9449d5b75e25c145f5a1f6efa1363823f8
r197566 = 49cc03e4bf77a42908478cfbda938088cf1a8567
r197567 = af1f071f0d7cca1b5b368272929370991d9a6402
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/922791
Comment 5 Konstantin Tokarev 2016-03-04 10:23:47 PST
Created attachment 273014 [details]
Patch
Comment 6 Michael Catanzaro 2016-03-04 11:39:25 PST
Comment on attachment 273014 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=273014&action=review

Amazing there are so few instances of this to change

> Source/WebCore/ChangeLog:18
> +        * page/DebugPageOverlays.cpp:

Got to remove this from the changelog.

The change to DebugPageOverlays.cpp was good, by the way; as discussed elsewhere, override and final are redundant. But I agree it doesn't need to be part of this patch.
Comment 7 Konstantin Tokarev 2016-03-04 13:21:51 PST
Comment on attachment 273014 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=273014&action=review

>> Source/WebCore/ChangeLog:18
>> +        * page/DebugPageOverlays.cpp:
> 
> Got to remove this from the changelog.
> 
> The change to DebugPageOverlays.cpp was good, by the way; as discussed elsewhere, override and final are redundant. But I agree it doesn't need to be part of this patch.

That changes just disappeared when I rebased it on top of Darin's patch :)
Comment 8 Konstantin Tokarev 2016-03-04 13:26:11 PST
Created attachment 273030 [details]
Patch
Comment 9 Konstantin Tokarev 2016-03-04 13:29:45 PST
(In reply to comment #6)
> Comment on attachment 273014 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=273014&action=review
> 
> Amazing there are so few instances of this to change

zandobersek have done a series of similar patches before, you can find them by words "missing override specifiers". Maybe we need to enforce policy with appropriate Werror.
Comment 10 Michael Catanzaro 2016-03-04 14:54:28 PST
(In reply to comment #9)
> zandobersek have done a series of similar patches before, you can find them
> by words "missing override specifiers". Maybe we need to enforce policy with
> appropriate Werror.

Bug #155049
Comment 11 WebKit Commit Bot 2016-03-04 15:28:14 PST
Comment on attachment 273030 [details]
Patch

Clearing flags on attachment: 273030

Committed r197591: <http://trac.webkit.org/changeset/197591>
Comment 12 WebKit Commit Bot 2016-03-04 15:28:17 PST
All reviewed patches have been landed.  Closing bug.