Bug 100995 - [Qt][Mac][Win] r133182 broke the build
Summary: [Qt][Mac][Win] r133182 broke the build
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Blocker
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 100907
  Show dependency treegraph
 
Reported: 2012-11-01 15:19 PDT by Nandor Huszka
Modified: 2012-11-03 04:13 PDT (History)
6 users (show)

See Also:


Attachments
proposed patch (1.38 KB, patch)
2012-11-01 15:23 PDT, Nandor Huszka
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
proposed patch (1.39 KB, patch)
2012-11-01 15:57 PDT, Nandor Huszka
buildbot: commit-queue-
Details | Formatted Diff | Diff
proposed patch (2.06 KB, patch)
2012-11-02 01:06 PDT, Nandor Huszka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nandor Huszka 2012-11-01 15:19:31 PDT
c:\webkitbuildslave\szeged-windows-1\qt-windows-32bit-release\build\source\webkit2\webprocess\webpage\coordinatedgraphics\CoordinatedGraphicsLayer.h(116) : error C3668: 'WebCore::CoordinatedGraphicsLayer::setContentsScale' : method with override specifier 'override' did not override any base class methods
c:\webkitbuildslave\szeged-windows-1\qt-windows-32bit-release\build\source\webkit2\webprocess\webpage\coordinatedgraphics\CoordinatedGraphicsLayer.h(117) : error C3668: 'WebCore::CoordinatedGraphicsLayer::setVisibleContentRectTrajectoryVector' : method with override specifier 'override' did not override any base class methods
Comment 1 Nandor Huszka 2012-11-01 15:23:13 PDT
Created attachment 171943 [details]
proposed patch
Comment 2 Early Warning System Bot 2012-11-01 15:47:53 PDT
Comment on attachment 171943 [details]
proposed patch

Attachment 171943 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14686645
Comment 3 Nandor Huszka 2012-11-01 15:57:47 PDT
Created attachment 171947 [details]
proposed patch

Fix pure virtual problem.
Comment 4 Build Bot 2012-11-01 16:30:05 PDT
Comment on attachment 171947 [details]
proposed patch

Attachment 171947 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14656691
Comment 5 Dongseong Hwang 2012-11-01 16:31:42 PDT
Thanks for reporting and patch. Who broke build is me.

I think we should not add this API in GraphicLayer.

I made a mistake to put virtual and OVERRIDE to two methods in CoordinatedGraphicsLayer.
    virtual void setContentsScale(float) OVERRIDE;
    virtual void setVisibleContentRectTrajectoryVector(const FloatPoint&) OVERRIDE;

How about removing virtual and OVERRIDE from two methods in CoordinatedGraphicsLayer like this.
    void setContentsScale(float);
    void setVisibleContentRectTrajectoryVector(const FloatPoint&);

I need two methods position in header file is also changed, because two methods mix with overrided methods of GraphicsLayer.

Could you do that?
Comment 6 Nandor Huszka 2012-11-02 00:37:11 PDT
(In reply to comment #5)
> Thanks for reporting and patch. Who broke build is me.
> 
> I think we should not add this API in GraphicLayer.
> 
> I made a mistake to put virtual and OVERRIDE to two methods in CoordinatedGraphicsLayer.
>     virtual void setContentsScale(float) OVERRIDE;
>     virtual void setVisibleContentRectTrajectoryVector(const FloatPoint&) OVERRIDE;
> 
> How about removing virtual and OVERRIDE from two methods in CoordinatedGraphicsLayer like this.
>     void setContentsScale(float);
>     void setVisibleContentRectTrajectoryVector(const FloatPoint&);
> 
> I need two methods position in header file is also changed, because two methods mix with overrided methods of GraphicsLayer.
> 
> Could you do that?

Thank you for the tip, of course I will do that. Unfortunately I chose the wrong way of fixing first. Now I see that other platforms do not need these methods.
Comment 7 Nandor Huszka 2012-11-02 01:06:56 PDT
Created attachment 172001 [details]
proposed patch
Comment 8 Dongseong Hwang 2012-11-02 01:40:52 PDT
(In reply to comment #7)
> Created an attachment (id=172001) [details]
> proposed patch

Thanks. LGTM! :)
Comment 9 Csaba Osztrogonác 2012-11-02 02:16:33 PDT
Comment on attachment 172001 [details]
proposed patch

Clearing flags on attachment: 172001

Committed r133276: <http://trac.webkit.org/changeset/133276>
Comment 10 Csaba Osztrogonác 2012-11-02 02:16:39 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Darin Adler 2012-11-02 15:57:38 PDT
Comment on attachment 172001 [details]
proposed patch

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

> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.h:126
> +    virtual void setContentsScale(float);
> +    virtual void setVisibleContentRectTrajectoryVector(const FloatPoint&);

The change log said you’d remove the virtual as well as OVERRIDE, but the patch removes OVERRIDE but leaves virtual in!
Comment 12 Dongseong Hwang 2012-11-02 20:51:09 PDT
(In reply to comment #11)
> (From update of attachment 172001 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=172001&action=review
> 
> > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.h:126
> > +    virtual void setContentsScale(float);
> > +    virtual void setVisibleContentRectTrajectoryVector(const FloatPoint&);
> 
> The change log said you’d remove the virtual as well as OVERRIDE, but the patch removes OVERRIDE but leaves virtual in!

Thanks. I filed Bug 101141.
Comment 13 Nandor Huszka 2012-11-03 04:13:42 PDT
(In reply to comment #11)
> The change log said you’d remove the virtual as well as OVERRIDE, but the patch removes OVERRIDE but leaves virtual in!

I forgot to delete it too, thank you for the remark!