Skip to content

Commit

Permalink
ScrollDirection -> ScrollAxis
Browse files Browse the repository at this point in the history
Change-Id: I5eea9b0747b39826a29cf2561357713303f2490a

Bug: 1385482
Change-Id: I5eea9b0747b39826a29cf2561357713303f2490a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4032767
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1073343}
  • Loading branch information
kevers-google authored and Chromium LUCI CQ committed Nov 18, 2022
1 parent 89add41 commit 4547c1e
Show file tree
Hide file tree
Showing 23 changed files with 136 additions and 224 deletions.
4 changes: 2 additions & 2 deletions third_party/blink/renderer/bindings/generated_in_core.gni
Original file line number Diff line number Diff line change
Expand Up @@ -540,10 +540,10 @@ generated_enumeration_sources_in_core = [
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_resize_quality.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_response_type.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_response_type.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_scroll_axis.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_scroll_axis.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_scroll_behavior.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_scroll_behavior.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_scroll_direction.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_scroll_direction.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_scroll_logical_position.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_scroll_logical_position.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_scroll_restoration.cc",
Expand Down
1 change: 0 additions & 1 deletion third_party/blink/renderer/bindings/idl_in_core.gni
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ static_idl_files_in_core = get_path_info(
"//third_party/blink/renderer/core/animation/keyframe_effect.idl",
"//third_party/blink/renderer/core/animation/keyframe_effect_options.idl",
"//third_party/blink/renderer/core/animation/optional_effect_timing.idl",
"//third_party/blink/renderer/core/animation/scroll_direction.idl",
"//third_party/blink/renderer/core/animation/scroll_timeline.idl",
"//third_party/blink/renderer/core/animation/scroll_timeline_options.idl",
"//third_party/blink/renderer/core/animation/view_timeline.idl",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,40 +17,38 @@ CSSScrollTimeline::Options::Options(
TimelineAxis axis)
: reference_type_(reference_type),
reference_element_(reference_element),
direction_(ComputeScrollDirection(axis)),
axis_(ComputeAxis(axis)),
name_(name) {}

ScrollTimeline::ScrollDirection
CSSScrollTimeline::Options::ComputeScrollDirection(TimelineAxis axis) {
using ScrollDirection = ScrollTimeline::ScrollDirection;

ScrollTimeline::ScrollAxis CSSScrollTimeline::Options::ComputeAxis(
TimelineAxis axis) {
switch (axis) {
case TimelineAxis::kBlock:
return ScrollDirection::kBlock;
return ScrollAxis::kBlock;
case TimelineAxis::kInline:
return ScrollDirection::kInline;
return ScrollAxis::kInline;
case TimelineAxis::kVertical:
return ScrollDirection::kVertical;
return ScrollAxis::kVertical;
case TimelineAxis::kHorizontal:
return ScrollDirection::kHorizontal;
return ScrollAxis::kHorizontal;
}

NOTREACHED();
return ScrollDirection::kBlock;
return ScrollAxis::kBlock;
}

CSSScrollTimeline::CSSScrollTimeline(Document* document, Options&& options)
: ScrollTimeline(document,
options.reference_type_,
options.reference_element_.value_or(
document->ScrollingElementNoLayout()),
options.direction_),
options.axis_),
name_(options.name_) {}

bool CSSScrollTimeline::Matches(const Options& options) const {
return (GetReferenceType() == options.reference_type_) &&
(ReferenceElement() == options.reference_element_) &&
(GetOrientation() == options.direction_) && (name_ == options.name_);
(GetAxis() == options.axis_) && (name_ == options.name_);
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,16 @@ class CORE_EXPORT CSSScrollTimeline : public ScrollTimeline {
ScrollTimeline::ReferenceType reference_type,
absl::optional<Element*> reference_element,
const AtomicString& name,
TimelineAxis);
TimelineAxis axis);

static ScrollTimeline::ScrollDirection ComputeScrollDirection(TimelineAxis);
static ScrollAxis ComputeAxis(TimelineAxis);

private:
friend class CSSScrollTimeline;

ScrollTimeline::ReferenceType reference_type_;
absl::optional<Element*> reference_element_;
ScrollTimeline::ScrollDirection direction_;
ScrollAxis axis_;
AtomicString name_;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,14 @@ CSSViewTimeline::Options::Options(Element* subject,
TimelineAxis axis,
TimelineInset inset)
: subject_(subject),
direction_(CSSScrollTimeline::Options::ComputeScrollDirection(axis)),
axis_(CSSScrollTimeline::Options::ComputeAxis(axis)),
inset_(inset.GetStart(), inset.GetEnd()) {}

CSSViewTimeline::CSSViewTimeline(Document* document, Options&& options)
: ViewTimeline(document,
options.subject_,
options.direction_,
options.inset_) {}
: ViewTimeline(document, options.subject_, options.axis_, options.inset_) {}

bool CSSViewTimeline::Matches(const Options& options) const {
return (subject() == options.subject_) &&
(GetOrientation() == options.direction_) &&
return (subject() == options.subject_) && (GetAxis() == options.axis_) &&
(GetInset() == options.inset_);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class CORE_EXPORT CSSViewTimeline : public ViewTimeline {
friend class CSSViewTimeline;

Element* subject_;
ScrollTimeline::ScrollDirection direction_;
ScrollAxis axis_;
ViewTimeline::Inset inset_;
};

Expand Down
12 changes: 0 additions & 12 deletions third_party/blink/renderer/core/animation/scroll_direction.idl

This file was deleted.

80 changes: 17 additions & 63 deletions third_party/blink/renderer/core/animation/scroll_timeline.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,17 @@ namespace blink {

namespace {

ScrollOrientation ToPhysicalScrollOrientation(
ScrollTimeline::ScrollDirection direction,
const LayoutBox& source_box) {
ScrollOrientation ToPhysicalScrollOrientation(ScrollAxis axis,
const LayoutBox& source_box) {
bool is_horizontal = source_box.IsHorizontalWritingMode();
switch (direction) {
case ScrollTimeline::ScrollDirection::kBlock:
switch (axis) {
case ScrollAxis::kBlock:
return is_horizontal ? kVerticalScroll : kHorizontalScroll;
case ScrollTimeline::ScrollDirection::kInline:
case ScrollAxis::kInline:
return is_horizontal ? kHorizontalScroll : kVerticalScroll;
case ScrollTimeline::ScrollDirection::kHorizontal:
case ScrollAxis::kHorizontal:
return kHorizontalScroll;
case ScrollTimeline::ScrollDirection::kVertical:
case ScrollAxis::kVertical:
return kVerticalScroll;
}
}
Expand All @@ -62,13 +61,8 @@ ScrollTimeline* ScrollTimeline::Create(Document& document,
? absl::make_optional(options->source())
: absl::nullopt;

// TODO(crbug.com/1060384): Update to axis in alignment with the spec rewrite.
ScrollDirection orientation;
if (!StringToScrollDirection(options->orientation(), orientation)) {
exception_state.ThrowDOMException(DOMExceptionCode::kNotSupportedError,
"Invalid orientation");
return nullptr;
}
ScrollAxis axis =
options->hasAxis() ? options->axis().AsEnum() : ScrollAxis::kBlock;

// The scrollingElement depends on style/layout-tree in quirks mode. Update
// such that subsequent calls to ScrollingElementNoLayout returns up-to-date
Expand All @@ -77,50 +71,28 @@ ScrollTimeline* ScrollTimeline::Create(Document& document,
document.UpdateStyleAndLayoutTree();

return Create(&document, source.value_or(document.ScrollingElementNoLayout()),
orientation);
axis);
}

ScrollTimeline* ScrollTimeline::Create(Document* document,
Element* source,
ScrollDirection orientation) {
ScrollAxis axis) {
ScrollTimeline* scroll_timeline = MakeGarbageCollected<ScrollTimeline>(
document, ReferenceType::kSource, source, orientation);
document, ReferenceType::kSource, source, axis);
scroll_timeline->UpdateSnapshot();

return scroll_timeline;
}

bool ScrollTimeline::StringToScrollDirection(
String scroll_direction,
ScrollTimeline::ScrollDirection& result) {
if (scroll_direction == "block") {
result = ScrollDirection::kBlock;
return true;
}
if (scroll_direction == "inline") {
result = ScrollDirection::kInline;
return true;
}
if (scroll_direction == "horizontal") {
result = ScrollDirection::kHorizontal;
return true;
}
if (scroll_direction == "vertical") {
result = ScrollDirection::kVertical;
return true;
}
return false;
}

ScrollTimeline::ScrollTimeline(Document* document,
ReferenceType reference_type,
Element* reference,
ScrollDirection orientation)
ScrollAxis axis)
: AnimationTimeline(document),
ScrollSnapshotClient(document->GetFrame()),
reference_type_(reference_type),
reference_element_(reference),
orientation_(orientation) {
axis_(axis) {
UpdateResolvedSource();
}

Expand Down Expand Up @@ -203,8 +175,7 @@ ScrollTimeline::TimelineState ScrollTimeline::ComputeTimelineState() {
scrollable_area->MinimumScrollOffset().x() == 0);

ScrollOffset scroll_offset = scrollable_area->GetScrollOffset();
auto physical_orientation =
ToPhysicalScrollOrientation(orientation_, *layout_box);
auto physical_orientation = ToPhysicalScrollOrientation(axis_, *layout_box);
double current_offset = (physical_orientation == kHorizontalScroll)
? scroll_offset.x()
: scroll_offset.y();
Expand Down Expand Up @@ -330,22 +301,6 @@ Element* ScrollTimeline::SourceInternal() const {
return nullptr;
}

String ScrollTimeline::orientation() {
switch (orientation_) {
case ScrollDirection::kBlock:
return "block";
case ScrollDirection::kInline:
return "inline";
case ScrollDirection::kHorizontal:
return "horizontal";
case ScrollDirection::kVertical:
return "vertical";
default:
NOTREACHED();
return "";
}
}

void ScrollTimeline::GetCurrentAndMaxOffset(const LayoutBox* layout_box,
double& current_offset,
double& max_offset) const {
Expand All @@ -369,8 +324,7 @@ void ScrollTimeline::GetCurrentAndMaxOffset(const LayoutBox* layout_box,
ScrollOffset scroll_dimensions = scrollable_area->MaximumScrollOffset() -
scrollable_area->MinimumScrollOffset();

auto physical_orientation =
ToPhysicalScrollOrientation(orientation_, *layout_box);
auto physical_orientation = ToPhysicalScrollOrientation(axis_, *layout_box);

if (physical_orientation == kHorizontalScroll) {
current_offset = scroll_offset.x();
Expand All @@ -381,7 +335,7 @@ void ScrollTimeline::GetCurrentAndMaxOffset(const LayoutBox* layout_box,
}
// When using a rtl direction, current_offset grows correctly from 0 to
// max_offset, but is negative. Since our offsets are all just deltas along
// the orientation direction, we can just take the absolute current_offset and
// the axis direction, we can just take the absolute current_offset and
// use that everywhere.
current_offset = std::abs(current_offset);
}
Expand Down
22 changes: 7 additions & 15 deletions third_party/blink/renderer/core/animation/scroll_timeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/gtest_prod_util.h"
#include "base/time/time.h"
#include "cc/animation/scroll_timeline.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_scroll_axis.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_typedefs.h"
#include "third_party/blink/renderer/core/animation/animation_timeline.h"
#include "third_party/blink/renderer/core/animation/timing.h"
Expand Down Expand Up @@ -40,13 +41,7 @@ class CORE_EXPORT ScrollTimeline : public AnimationTimeline,

public:
using ScrollOffsets = cc::ScrollTimeline::ScrollOffsets;

enum class ScrollDirection {
kBlock,
kInline,
kHorizontal,
kVertical,
};
using ScrollAxis = V8ScrollAxis::Enum;

// Indicates the relation between the reference element and source of the
// scroll timeline.
Expand All @@ -62,18 +57,15 @@ class CORE_EXPORT ScrollTimeline : public AnimationTimeline,

static ScrollTimeline* Create(Document* document,
Element* source,
ScrollDirection orientation);
ScrollAxis axis);

// Construct ScrollTimeline objects through one of the Create methods, which
// perform initial snapshots, as it can't be done during the constructor due
// to possibly depending on overloaded functions.
ScrollTimeline(Document*,
ReferenceType reference_type,
Element* reference,
ScrollDirection);

static bool StringToScrollDirection(String scroll_direction,
ScrollTimeline::ScrollDirection& result);
ScrollAxis axis);

bool IsScrollTimeline() const override { return true; }
// ScrollTimeline is not active if source is null, does not currently
Expand All @@ -90,7 +82,7 @@ class CORE_EXPORT ScrollTimeline : public AnimationTimeline,

// IDL API implementation.
Element* source() const;
String orientation();
const V8ScrollAxis axis() const { return V8ScrollAxis(axis_); }

V8CSSNumberish* currentTime() override;
V8CSSNumberish* duration() override;
Expand All @@ -106,7 +98,7 @@ class CORE_EXPORT ScrollTimeline : public AnimationTimeline,
// timeline is inactive.
absl::optional<ScrollOffsets> GetResolvedScrollOffsets() const;

ScrollDirection GetOrientation() const { return orientation_; }
ScrollAxis GetAxis() const { return axis_; }

void GetCurrentAndMaxOffset(const LayoutBox*,
double& current_offset,
Expand Down Expand Up @@ -194,7 +186,7 @@ class CORE_EXPORT ScrollTimeline : public AnimationTimeline,
ReferenceType reference_type_;
Member<Element> reference_element_;
Member<Node> resolved_source_;
ScrollDirection orientation_;
ScrollAxis axis_;

// Snapshotted value produced by the last SnapshotState call.
TimelineState timeline_state_snapshotted_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

enum ScrollAxis { "block", "inline", "horizontal", "vertical" };

// https://wicg.github.io/scroll-animations/#scrolltimeline-interface
[
RuntimeEnabled=ScrollTimeline,
Exposed=Window
] interface ScrollTimeline : AnimationTimeline {
[CallWith=Document, RaisesException, MeasureAs=ScrollTimelineConstructor] constructor(optional ScrollTimelineOptions options = {});
readonly attribute Element? source;
readonly attribute ScrollDirection orientation;
readonly attribute ScrollAxis axis;
};
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@

dictionary ScrollTimelineOptions {
Element? source;
ScrollDirection orientation = "block";
ScrollAxis axis = "block";
};
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class TestScrollTimeline : public ScrollTimeline {
: ScrollTimeline(document,
ScrollTimeline::ReferenceType::kSource,
source,
ScrollDirection::kVertical) {
ScrollAxis::kVertical) {
UpdateSnapshot();
}

Expand Down Expand Up @@ -164,7 +164,7 @@ TEST_F(ScrollTimelineTest, AttachOrDetachAnimationWithNullSource) {
// documentElement from the document.
Element* scroll_source = nullptr;
Persistent<ScrollTimeline> scroll_timeline = ScrollTimeline::Create(
&GetDocument(), scroll_source, ScrollTimeline::ScrollDirection::kBlock);
&GetDocument(), scroll_source, ScrollTimeline::ScrollAxis::kBlock);

// Sanity checks.
ASSERT_EQ(scroll_timeline->source(), nullptr);
Expand Down

0 comments on commit 4547c1e

Please sign in to comment.