Bug 42236 – [Chromium] Replace ChromiumBridge::widgetSetCursor with ChromeClient::setCursor

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Darin Fisher (:fishd, Google)]

Replace ChromiumBridge::widgetSetCursor with ChromeClient::setCursor

[Adam Klein]

Request for some clarification: the only call to ChromiumBridge::widgetSetCursor() is in Widget::setCursor() (WidgetChromium.cpp). But Widget itself doesn't have easy access to a ChromeClient.  Is the intention just to move ChromiumBridge's logic up into WidgetChromium (it involves using pseudo-RTTI and static_cast<FrameView*>(this)), or did you have something else in mind?

[Adam Klein]

Oh, I see that Widget actually has a nice root() method which does this for me.  Using that seems to be the way to go.

[Darin Fisher (:fishd, Google)]

Yes, you can see in ChromiumBridge.cpp how we get to the ChromeClientImpl class given only a Widget.  It used to be the case that ChromeClient did not have the setCursor method; however, it was added for WebKit2 (in r63339).  We can use ChromeClient from our WidgetChromium implementation to reach the new setCursor method.  We could not previously cast to ChromeClientImpl from WidgetChromium because WebCore cannot depend directly on code in WebKit.

[Adam Klein]

Created attachment 79024 [details]
Patch

[Darin Fisher (:fishd, Google)]

Comment on attachment 79024 [details]
Patch

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

> Source/WebCore/platform/chromium/WidgetChromium.cpp:63
> +    FrameView* view;

Sorry, I just realized there is a much simpler way to achieve this!

HostWindow also gained a setCursor method, and you can get the HostWindow
from a Widget's parent ScrollView.  That will dramatically simplify this
code.

[Darin Fisher (:fishd, Google)]

The HostWindow approach makes more sense too because there is a "rule" that WebCore/platform/ should not depend back onto code outside of WebCore/platform/ (with the exception of wtf/).

[Adam Klein]

Created attachment 79075 [details]
Patch

[Adam Klein]

Indeed, much simpler.  And it matches with the other platforms do.

[WebKit Commit Bot]

Comment on attachment 79075 [details]
Patch

Clearing flags on attachment: 79075

Committed r76048: <http://trac.webkit.org/changeset/76048>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.