Bug 88597 – [chromium] Port Canvas2DLayerBridge over to WebExternalTextureLayer

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

[chromium] Port Canvas2DLayerBridge over to WebExternalTextureLayer

[James Robinson]

Created attachment 146429 [details]
Patch

[James Robinson]

Created attachment 146984 [details]
rebased and fleshed out ChangeLogs

[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 146984 [details]
rebased and fleshed out ChangeLogs

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

> Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:91
>          MockTextureCopier copierMock;

Unused now?

> Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:-117
> -            EXPECT_CALL(implMock, flush());

Why remove the implMock expectations?

[James Robinson]

(In reply to comment #4)
> (From update of attachment 146984 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=146984&action=review
> 
> > Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:91
> >          MockTextureCopier copierMock;
> 
> Unused now?

Indeed - will remove

> 
> > Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:-117
> > -            EXPECT_CALL(implMock, flush());
> 
> Why remove the implMock expectations?

The flush this mock was checking for was coming from the updater's implementation.  The test no longer verifies the updater's implementation, it mocks the updater itself and verifies that we make the appropriate call (there is (hopefully) sufficient test coverage that the updater DTRT elsewhere).

[Adrienne Walker]

Comment on attachment 146984 [details]
rebased and fleshed out ChangeLogs

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 146984 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=146984&action=review
> > 
> > > Source/WebKit/chromium/tests/Canvas2DLayerBridgeTest.cpp:-117
> > > -            EXPECT_CALL(implMock, flush());
> > 
> > Why remove the implMock expectations?
> 
> The flush this mock was checking for was coming from the updater's implementation.  The test no longer verifies the updater's implementation, it mocks the updater itself and verifies that we make the appropriate call (there is (hopefully) sufficient test coverage that the updater DTRT elsewhere).

Ohhhh, quite right.  R=me, then.

[James Robinson]

Created attachment 147157 [details]
Patch

[James Robinson]

Removed the unused MockTextureCopier and ported the fix of http://trac.webkit.org/changeset/120117

[Adrienne Walker]

Comment on attachment 147157 [details]
Patch

R=me.  Sounds good.

[WebKit Review Bot]

Comment on attachment 147157 [details]
Patch

Clearing flags on attachment: 147157

Committed r120142: <http://trac.webkit.org/changeset/120142>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.