Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(115)

Issue 2389323003: Test type of exposed AnimationEffectTimingReadOnly (Closed)

Created:
4 years, 2 months ago by suzyh_UTC10 (ex-contributor)
Modified:
4 years, 2 months ago
CC:
darktears, blink-reviews, blink-reviews-animation_chromium.org, chromium-reviews, Eric Willigers, rjwright, shans
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Test type of exposed AnimationEffectTimingReadOnly The KeyframeEffectReadOnly interface was implemented in https://codereview.chromium.org/2369833002 as a simple redirection to a KeyframeEffect object (where KeyframeEffect is actually a subclass of KeyframeEffectReadOnly). In a subsequent patch (https://codereview.chromium.org/2379863003), the timing attribute of KeyframeEffect was changed: KeyframeEffectReadOnly.timing is an AnimationEffectTimingReadOnly, and KeyframeEffect.timing is an AnimationEffectTiming (a subclass of AnimationEffectTimingReadOnly). It turns out that if you construct a KeyframeEffectReadOnly object in JavaScript, the V8/JavaScript interface magically interprets and represents the underlying KeyframeEffect object as a KeyframeEffectReadOnly object, as we wanted. But when the JavaScript then accesses the timing attribute, it appears to treat it as a KeyframeEffect object, calling the KeyframeEffect version of the getter instead of the KeyframeEffectReadOnly version. This patch commits a test that illustrates the bug. A future patch will fix the bug by actually constructing KeyframeEffectReadOnly objects in the C++. BUG=624639 Committed: https://crrev.com/b6d80c681fbc035f6267353b4fefbab1b33744ae Cr-Commit-Position: refs/heads/master@{#423115}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -0 lines) Patch
A third_party/WebKit/LayoutTests/animations/api-readonly-object-types.html View 1 chunk +28 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/animations/api-readonly-object-types-expected.txt View 1 chunk +5 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 8 (3 generated)
suzyh_UTC10 (ex-contributor)
Let's get this in while I work on the fix.
4 years, 2 months ago (2016-10-05 07:20:13 UTC) #2
alancutter (OOO until 2018)
lgtm
4 years, 2 months ago (2016-10-05 07:28:50 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2389323003/1
4 years, 2 months ago (2016-10-05 07:39:25 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 2 months ago (2016-10-05 08:35:41 UTC) #6
commit-bot: I haz the power
4 years, 2 months ago (2016-10-05 08:36:59 UTC) #8
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/b6d80c681fbc035f6267353b4fefbab1b33744ae
Cr-Commit-Position: refs/heads/master@{#423115}

Powered by Google App Engine
This is Rietveld 408576698