Bug 100995 – [Qt][Mac][Win] r133182 broke the build

• Component: New Bugs
• Status: RESOLVED FIXED

[Nandor Huszka]

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

[Nandor Huszka]

Created attachment 171943 [details]
proposed patch

[Early Warning System Bot]

Comment on attachment 171943 [details]
proposed patch

Attachment 171943 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14686645

[Nandor Huszka]

Created attachment 171947 [details]
proposed patch

Fix pure virtual problem.

[Build Bot]

Comment on attachment 171947 [details]
proposed patch

Attachment 171947 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14656691

[Dongseong Hwang]

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?

[Nandor Huszka]

(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.

[Nandor Huszka]

Created attachment 172001 [details]
proposed patch

[Dongseong Hwang]

(In reply to comment #7)
> Created an attachment (id=172001) [details]
> proposed patch

Thanks. LGTM! :)

[Csaba Osztrogonác]

Comment on attachment 172001 [details]
proposed patch

Clearing flags on attachment: 172001

Committed r133276: <http://trac.webkit.org/changeset/133276>

[Csaba Osztrogonác]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

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!

[Dongseong Hwang]

(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.

[Nandor Huszka]

(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!