Bug 95679 - [chromium] Wire up scrollable sublayers in ScrollingCoordinatorChromium
Summary: [chromium] Wire up scrollable sublayers in ScrollingCoordinatorChromium
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sami Kyöstilä
URL:
Keywords:
Depends on: 78862
Blocks: 66687
  Show dependency treegraph
 
Reported: 2012-09-03 04:26 PDT by Sami Kyöstilä
Modified: 2012-09-05 10:25 PDT (History)
9 users (show)

See Also:


Attachments
Patch (24.02 KB, patch)
2012-09-03 08:02 PDT, Sami Kyöstilä
no flags Details | Formatted Diff | Diff
Patch (25.64 KB, patch)
2012-09-05 04:29 PDT, Sami Kyöstilä
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sami Kyöstilä 2012-09-03 04:26:02 PDT
We should implement scrollable sublayers in ScrollingCoordinatorChromium.
Comment 1 Sami Kyöstilä 2012-09-03 08:02:40 PDT
Created attachment 161929 [details]
Patch

Patch (requires 78862).
Comment 2 James Robinson 2012-09-04 12:26:06 PDT
Comment on attachment 161929 [details]
Patch

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

> Source/WebKit/chromium/tests/GraphicsLayerChromiumTest.cpp:132
> +    MOCK_CONST_METHOD0(isActive, bool());

I don't think you really need a mock here at all - you just need a fake.  I'd drop the gmock macros, etc, and just have empty implementations of the pure virtuals

> Source/WebKit/chromium/tests/WebLayerTest.cpp:185
> +        m_didScrollCalled = true;

here, on the flip side, you've basically hand-rolled gmock's EXPECT_CALL :). I would use the gmock macros here since it's a small pure virtual interface
Comment 3 Sami Kyöstilä 2012-09-05 04:29:49 PDT
Created attachment 162216 [details]
Patch

- Added missing WebLayerScrollClient.h.
- Not rolling my own test infrastructure :)
Comment 4 WebKit Review Bot 2012-09-05 04:35:13 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 5 James Robinson 2012-09-05 09:18:20 PDT
Comment on attachment 162216 [details]
Patch

Great!
Comment 6 WebKit Review Bot 2012-09-05 10:25:20 PDT
Comment on attachment 162216 [details]
Patch

Clearing flags on attachment: 162216

Committed r127605: <http://trac.webkit.org/changeset/127605>
Comment 7 WebKit Review Bot 2012-09-05 10:25:23 PDT
All reviewed patches have been landed.  Closing bug.