Bug 96151 – [chromium] Implement WebCompositorInputHandlerImpl on top of exposed API instead of CC internals

• Component: New Bugs
• Status: RESOLVED FIXED

[James Robinson]

[chromium] Implement WebCompositorInputHandlerImpl on top of exposed API instead of CC internals

[James Robinson]

Created attachment 162890 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Adrienne Walker]

Comment on attachment 162890 [details]
Patch

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

There's a lot of code change, but it seems pretty straightforward.

> Source/WebKit/chromium/src/WebCompositorInputHandlerImpl.cpp:237
> +    TRACE_EVENT_INSTANT0("cc", "WebCompositorInputHandlerImpl::bindToClient");
> +    if (!s_compositors)
> +        s_compositors = new HashSet<WebCompositorInputHandlerImpl*>;
> +    s_compositors->add(this);

Now that this doesn't happen on construction, do you want some check that bindToClient is only called once?

> Source/WebKit/chromium/src/WebToCCInputHandlerAdapter.cpp:60
> +        return static_cast<WebInputHandlerClient::ScrollStatus>(m_client->scrollBegin(point, static_cast<WebCore::CCInputHandlerClient::ScrollInputType>(type)));

Can you add compile-time asserts that these enums are the same?

[James Robinson]

(In reply to comment #3)
> (From update of attachment 162890 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162890&action=review
> 
> There's a lot of code change, but it seems pretty straightforward.
> 
> > Source/WebKit/chromium/src/WebCompositorInputHandlerImpl.cpp:237
> > +    TRACE_EVENT_INSTANT0("cc", "WebCompositorInputHandlerImpl::bindToClient");
> > +    if (!s_compositors)
> > +        s_compositors = new HashSet<WebCompositorInputHandlerImpl*>;
> > +    s_compositors->add(this);
> 
> Now that this doesn't happen on construction, do you want some check that bindToClient is only called once?

Yeah, good idea.

> 
> > Source/WebKit/chromium/src/WebToCCInputHandlerAdapter.cpp:60
> > +        return static_cast<WebInputHandlerClient::ScrollStatus>(m_client->scrollBegin(point, static_cast<WebCore::CCInputHandlerClient::ScrollInputType>(type)));
> 
> Can you add compile-time asserts that these enums are the same?

Sure - just have to figure out how those macros work

[James Robinson]

Created attachment 162905 [details]
checks bindToClient called once, asserts enums match, puts WCIHI in correct gyp section

[Adrienne Walker]

Comment on attachment 162905 [details]
checks bindToClient called once, asserts enums match, puts WCIHI in correct gyp section

R=me.

[James Robinson]

Committed r127948: <http://trac.webkit.org/changeset/127948>