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

Unified Diff: third_party/WebKit/Source/core/html/HTMLMarqueeElement.cpp

Issue 2549443003: Move <marquee> implementation to HTMLMarqueeElement.cpp (Closed)
Patch Set: Move out transform string creation into separate method Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/html/HTMLMarqueeElement.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLMarqueeElement.cpp b/third_party/WebKit/Source/core/html/HTMLMarqueeElement.cpp
index 1da81e76d6b184d3c023742cc8a3cde9eea043b3..da35d765e554d79963767d9d48a19934fc815f33 100644
--- a/third_party/WebKit/Source/core/html/HTMLMarqueeElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLMarqueeElement.cpp
@@ -22,32 +22,65 @@
#include "core/html/HTMLMarqueeElement.h"
-#include "bindings/core/v8/PrivateScriptRunner.h"
+#include "bindings/core/v8/ExceptionStatePlaceholder.h"
#include "bindings/core/v8/V8HTMLMarqueeElement.h"
+#include "core/CSSPropertyNames.h"
#include "core/HTMLNames.h"
+#include "core/animation/DocumentTimeline.h"
+#include "core/animation/KeyframeEffect.h"
+#include "core/animation/KeyframeEffectModel.h"
+#include "core/animation/KeyframeEffectOptions.h"
+#include "core/animation/StringKeyframe.h"
+#include "core/animation/TimingInput.h"
+#include "core/css/CSSStyleDeclaration.h"
#include "core/dom/Document.h"
+#include "core/dom/shadow/ShadowRoot.h"
+#include "core/frame/LocalDOMWindow.h"
#include "core/frame/UseCounter.h"
-#include "platform/ScriptForbiddenScope.h"
+#include "core/html/HTMLDimension.h"
+#include <cstdlib>
namespace blink {
+namespace {
+
+String convertHTMLLengthToCSSLength(const String& htmlLength) {
+ HTMLDimension dimension;
+ parseDimensionValue(htmlLength, dimension);
+ if (dimension.isRelative())
+ return String();
+ CSSPrimitiveValue* cssValue = CSSPrimitiveValue::create(
+ dimension.value(), dimension.isPercentage()
+ ? CSSPrimitiveValue::UnitType::Percentage
+ : CSSPrimitiveValue::UnitType::Pixels);
+ return cssValue->customCSSText();
+}
+
+} // namespace
+
inline HTMLMarqueeElement::HTMLMarqueeElement(Document& document)
: HTMLElement(HTMLNames::marqueeTag, document) {
- if (document.contextDocument()) {
- ScriptForbiddenScope::AllowUserAgentScript script;
- v8::Local<v8::Value> classObject =
- PrivateScriptRunner::installClassIfNeeded(&document,
- "HTMLMarqueeElement");
- RELEASE_ASSERT(!classObject.IsEmpty());
- }
UseCounter::count(document, UseCounter::HTMLMarqueeElement);
+ ShadowRoot* shadow =
+ createShadowRootInternal(ShadowRootType::V0, ASSERT_NO_EXCEPTION);
jbroman 2016/12/01 22:22:52 Ultimately this should probably be a user agent sh
adithyas 2016/12/02 19:29:49 Yup, I'd prefer to do this is in a follow-up, espe
+ Element* style = document.createElement("style");
+ style->setTextContent(
+ ":host { display: inline-block; width: -webkit-fill-available; overflow: "
+ "hidden; text-align: initial; white-space: nowrap; }"
+ ":host([direction=\"up\"]), :host([direction=\"down\"]) { overflow: "
+ "initial; overflow-y: hidden; white-space: initial; }"
+ ":host > div { will-change: transform; }");
adithyas 2016/12/01 20:30:48 "will-change: transform" is added in to force an a
jbroman 2016/12/01 22:22:52 To elaborate slightly, this layer gets created any
+ shadow->appendChild(style);
+
+ Element* mover = document.createElement("div");
+ shadow->appendChild(mover);
+
+ mover->appendChild(document.createElement("content"));
+ m_mover = mover;
}
HTMLMarqueeElement* HTMLMarqueeElement::create(Document& document) {
- HTMLMarqueeElement* marqueeElement = new HTMLMarqueeElement(document);
- V8HTMLMarqueeElement::PrivateScript::createdCallbackMethod(document.frame(),
- marqueeElement);
- return marqueeElement;
+ return new HTMLMarqueeElement(document);
}
void HTMLMarqueeElement::attributeChanged(const QualifiedName& name,
@@ -55,16 +88,22 @@ void HTMLMarqueeElement::attributeChanged(const QualifiedName& name,
const AtomicString& newValue,
AttributeModificationReason reason) {
HTMLElement::attributeChanged(name, oldValue, newValue, reason);
- V8HTMLMarqueeElement::PrivateScript::attributeChangedCallbackMethod(
- document().frame(), this, name.toString(), oldValue, newValue);
+ attributeChangedCallback(name, newValue);
}
Node::InsertionNotificationRequest HTMLMarqueeElement::insertedInto(
ContainerNode* insertionPoint) {
HTMLElement::insertedInto(insertionPoint);
+
if (isConnected()) {
- V8HTMLMarqueeElement::PrivateScript::attachedCallbackMethod(
- document().frame(), this);
+ std::vector<QualifiedName> presentationalAttributes = {
jbroman 2016/12/01 22:22:52 as a rule we prefer WTF::Vector to std::vector ins
adithyas 2016/12/02 19:29:49 Fixed.
+ HTMLNames::bgcolorAttr, HTMLNames::heightAttr, HTMLNames::hspaceAttr,
+ HTMLNames::vspaceAttr, HTMLNames::widthAttr};
+ for (auto attr : presentationalAttributes) {
+ initializeAttribute(attr);
+ }
+
+ this->start();
jbroman 2016/12/01 22:22:52 nit: in C++, prefer "start();" to "this->start();"
adithyas 2016/12/02 19:29:49 Fixed, here and everywhere else.
}
return InsertionDone;
}
@@ -72,14 +111,413 @@ Node::InsertionNotificationRequest HTMLMarqueeElement::insertedInto(
void HTMLMarqueeElement::removedFrom(ContainerNode* insertionPoint) {
HTMLElement::removedFrom(insertionPoint);
if (insertionPoint->isConnected()) {
- V8HTMLMarqueeElement::PrivateScript::detachedCallbackMethod(
- insertionPoint->document().frame(), this);
+ this->stop();
}
}
bool HTMLMarqueeElement::isHorizontal() const {
- AtomicString direction = getAttribute(HTMLNames::directionAttr);
- return direction != "down" && direction != "up";
+ Direction direction = this->direction();
+ return direction != Up && direction != Down;
+}
+
+int HTMLMarqueeElement::scrollAmount() {
+ bool ok;
+ int scrollAmount = getAttribute(HTMLNames::scrollamountAttr).toInt(&ok);
+ if (!ok || scrollAmount < 0) {
jbroman 2016/12/01 22:22:52 not-really-a-nit: Here and elsewhere, you're free
adithyas 2016/12/02 19:29:49 Removed the braces everywhere I didn't need them.
+ return kDefaultScrollAmount;
+ }
+ return scrollAmount;
+}
+
+void HTMLMarqueeElement::setScrollAmount(int value,
+ ExceptionState& exceptionState) {
+ if (value < 0) {
+ exceptionState.throwDOMException(
jbroman 2016/12/01 22:22:52 Here and elsewhere: ExceptionState::throwDOMExcept
adithyas 2016/12/02 19:29:49 Forgot about that! Fixed.
+ IndexSizeError,
+ "The provided value (" + String::number(value) + ") is negative.");
+ }
+ setIntegralAttribute(HTMLNames::scrollamountAttr, value);
+}
+
+int HTMLMarqueeElement::scrollDelay() {
+ bool ok;
+ int scrollDelay = getAttribute(HTMLNames::scrolldelayAttr).toInt(&ok);
+ if (!ok || scrollDelay < 0) {
+ return kDefaultScrollDelayMS;
+ }
+ return scrollDelay;
+}
+
+void HTMLMarqueeElement::setScrollDelay(int value,
+ ExceptionState& exceptionState) {
+ if (value < 0) {
+ exceptionState.throwDOMException(
+ IndexSizeError,
+ "The provided value (" + String::number(value) + ") is negative.");
+ }
+ setIntegralAttribute(HTMLNames::scrolldelayAttr, value);
+}
+
+int HTMLMarqueeElement::loop() {
+ bool ok;
+ int loop = getAttribute(HTMLNames::loopAttr).toInt(&ok);
+ if (!ok || loop <= 0) {
+ return kDefaultLoopLimit;
+ }
+ return loop;
+}
+
+void HTMLMarqueeElement::setLoop(int value, ExceptionState& exceptionState) {
+ if (value <= 0 && value != -1) {
+ exceptionState.throwDOMException(
+ IndexSizeError, "The provided value (" + String::number(value) +
+ ") is neither positive nor -1.");
+ }
+ setIntegralAttribute(HTMLNames::loopAttr, value);
+}
+
+void HTMLMarqueeElement::start() {
+ // User script must not run in a SVGImage, but it's okay to run user
+ // agent's script such as <marquee>. However, a function scheduled with
+ // requestAnimationFrame is indistinguishable if it's scheduled by user
+ // script or user agent's script. Thus we disallow scheduling a task
+ // in svg (not limited to SVGImage) entirely.
+ if (document().hasSVGRootNode())
jbroman 2016/12/01 22:22:52 I wonder if this still applies. Keeping it for now
adithyas 2016/12/02 19:29:49 Removed it, it's not necessary anymore.
+ return;
+
+ if (m_continueCallbackRequestId) {
+ return;
+ }
+ RequestAnimationFrameCallback* callback =
+ new RequestAnimationFrameCallback(this);
+ m_continueCallbackRequestId = document().requestAnimationFrame(callback);
+}
+
+void HTMLMarqueeElement::stop() {
+ if (m_continueCallbackRequestId) {
+ document().cancelAnimationFrame(m_continueCallbackRequestId);
+ m_continueCallbackRequestId = 0;
+ return;
+ }
+
+ if (m_player) {
+ m_player->pause();
+ }
+}
+
+void HTMLMarqueeElement::initializeAttribute(const QualifiedName& attr) {
+ const AtomicString& value = getAttribute(attr);
+ if (value.isNull())
+ return;
+ attributeChangedCallback(attr, value);
+}
+
+void HTMLMarqueeElement::attributeChangedCallback(const QualifiedName& attr,
+ const String& newValue) {
+ if (attr == HTMLNames::bgcolorAttr) {
+ style()->setProperty("bgcolor", newValue, String(), ASSERT_NO_EXCEPTION);
jbroman 2016/12/01 22:22:52 The CSS property is background-color, not bgcolor.
adithyas 2016/12/02 19:29:49 You are correct, they do use the hyphenated names,
+ } else if (attr == HTMLNames::heightAttr) {
+ style()->setProperty("height", convertHTMLLengthToCSSLength(newValue),
+ String(), ASSERT_NO_EXCEPTION);
+ } else if (attr == HTMLNames::hspaceAttr) {
+ style()->setProperty("marginLeft", convertHTMLLengthToCSSLength(newValue),
+ String(), ASSERT_NO_EXCEPTION);
+ style()->setProperty("marginRight", convertHTMLLengthToCSSLength(newValue),
+ String(), ASSERT_NO_EXCEPTION);
+ } else if (attr == HTMLNames::vspaceAttr) {
+ style()->setProperty("marginTop", convertHTMLLengthToCSSLength(newValue),
+ String(), ASSERT_NO_EXCEPTION);
+ style()->setProperty("marginBottom", convertHTMLLengthToCSSLength(newValue),
+ String(), ASSERT_NO_EXCEPTION);
+ } else if (attr == HTMLNames::widthAttr) {
+ style()->setProperty("width", convertHTMLLengthToCSSLength(newValue),
+ String(), ASSERT_NO_EXCEPTION);
+ }
+}
+
+void HTMLMarqueeElement::RequestAnimationFrameCallback::handleEvent(double) {
+ m_marquee->m_continueCallbackRequestId = 0;
+ m_marquee->continueAnimation();
+}
+
+void HTMLMarqueeElement::AnimationFinished::handleEvent(
+ ExecutionContext* context,
+ Event* event) {
+ ++m_marquee->m_loopCount;
+ m_marquee->start();
+}
+
+StringKeyframeEffectModel* HTMLMarqueeElement::createEffectModel(
+ AnimationParameters& parameters) {
+ StringKeyframeVector keyframes;
+ StyleSheetContents* styleSheetContents =
+ m_mover->document().elementSheet().contents();
+
+ RefPtr<StringKeyframe> keyframe1 = StringKeyframe::create();
+ keyframe1->setCSSPropertyValue(CSSPropertyTransform,
+ parameters.transformBegin, styleSheetContents);
+ keyframes.append(keyframe1);
jbroman 2016/12/01 22:22:52 nit: when passing ownership of a RefPtr, prefer to
adithyas 2016/12/02 19:29:49 Fixed! I'm going to keep using StringKeyFrame unle
+
+ RefPtr<StringKeyframe> keyframe2 = StringKeyframe::create();
+ keyframe2->setCSSPropertyValue(CSSPropertyTransform, parameters.transformEnd,
+ styleSheetContents);
+ keyframes.append(keyframe2);
+
+ StringKeyframeEffectModel* effectModel = StringKeyframeEffectModel::create(
+ keyframes, LinearTimingFunction::shared());
+ return effectModel;
jbroman 2016/12/01 22:22:52 super-nit: might as well just return StringKeyfra
adithyas 2016/12/02 19:29:49 Fixed.
+}
+
+// static
+void HTMLMarqueeElement::initializeTiming(Timing& timing, double duration) {
jbroman 2016/12/01 22:22:51 You can just return Timing from this method. Or be
adithyas 2016/12/02 19:29:49 removed the method
+ TimingInput::setFillMode(timing, "forwards");
jbroman 2016/12/01 22:22:52 I'd just use the enum etc here instead of converti
adithyas 2016/12/02 19:29:49 I've changed it to use the enum directly for fill
jbroman 2016/12/03 23:47:31 Ah, OK.
+ UnrestrictedDoubleOrString iterDuration;
+ iterDuration.setUnrestrictedDouble(duration);
+ TimingInput::setIterationDuration(timing, iterDuration, ASSERT_NO_EXCEPTION);
+}
+
+void HTMLMarqueeElement::continueAnimation() {
+ if (!shouldContinue()) {
+ return;
+ }
+ if (m_player && m_player->playState() == "paused") {
+ m_player->play();
+ return;
+ }
+
+ AnimationParameters parameters = getAnimationParameters();
+ int scrollDelay = this->scrollDelay();
+ int scrollAmount = this->scrollAmount();
+
+ if (scrollDelay < kMinimumScrollDelayMS && !trueSpeed())
+ scrollDelay = kDefaultScrollDelayMS;
+ double duration = 0;
+ if (scrollAmount)
+ duration = parameters.distance * scrollDelay / scrollAmount;
+ if (!duration)
+ return;
+
+ StringKeyframeEffectModel* effectModel = createEffectModel(parameters);
+ Timing timing;
+ initializeTiming(timing, duration);
+ KeyframeEffect* keyframeEffect =
+ KeyframeEffect::create(m_mover, effectModel, timing);
+ Animation* player = m_mover->document().timeline().play(keyframeEffect);
+ player->setId("");
jbroman 2016/12/01 22:22:52 nit: "player->setId(emptyString);" is slightly mor
adithyas 2016/12/02 19:29:49 Fixed (assuming you meant emptyString())
jbroman 2016/12/03 23:47:31 Indeed.
+ player->setOnfinish(new AnimationFinished(this));
+
+ m_player = player;
+}
+
+bool HTMLMarqueeElement::shouldContinue() {
+ int lp = loop();
+
+ // By default, slide loops only once.
+ if (lp <= 0 && behavior() == Slide)
+ lp = 1;
+
+ if (lp <= 0)
+ return true;
+ return m_loopCount < lp;
+}
+
+HTMLMarqueeElement::Behavior HTMLMarqueeElement::behavior() const {
+ const AtomicString& bhvr = getAttribute(HTMLNames::behaviorAttr);
+ if (bhvr == "alternate")
+ return Alternate;
+ if (bhvr == "slide")
+ return Slide;
+ return Scroll;
+}
+
+HTMLMarqueeElement::Direction HTMLMarqueeElement::direction() const {
+ const AtomicString& dir = getAttribute(HTMLNames::directionAttr);
+ if (dir == "down")
+ return Down;
+ if (dir == "up")
+ return Up;
+ if (dir == "right")
+ return Right;
+ return Left;
+}
+
+bool HTMLMarqueeElement::trueSpeed() const {
+ return hasAttribute(HTMLNames::truespeedAttr);
+}
+
+HTMLMarqueeElement::Metrics HTMLMarqueeElement::getMetrics() {
+ Metrics metrics;
+ CSSStyleDeclaration* marqueeStyle =
+ document().domWindow()->getComputedStyle(this, String());
+
+ // For marquees that are declared inline, getComputedStyle returns "auto" for
+ // width and height. Setting all the metrics to zero disables animation for
+ // inline marquees.
+ if (marqueeStyle->getPropertyValue("width") == "auto" &&
+ marqueeStyle->getPropertyValue("height") == "auto") {
+ metrics.contentHeight = 0;
+ metrics.contentWidth = 0;
+ metrics.marqueeWidth = 0;
+ metrics.marqueeHeight = 0;
+ return metrics;
+ }
+
+ if (isHorizontal()) {
+ m_mover->style()->setProperty("width", "-webkit-max-content", "important",
+ ASSERT_NO_EXCEPTION);
+
+ } else {
+ m_mover->style()->setProperty("height", "-webkit-max-content", "important",
+ ASSERT_NO_EXCEPTION);
+ }
+
+ CSSStyleDeclaration* moverStyle =
+ document().domWindow()->getComputedStyle(m_mover, String());
+
+ metrics.contentWidth = moverStyle->getPropertyValue("width").toDouble();
+ metrics.contentHeight = moverStyle->getPropertyValue("height").toDouble();
+ metrics.marqueeWidth = marqueeStyle->getPropertyValue("width").toDouble();
+ metrics.marqueeHeight = marqueeStyle->getPropertyValue("height").toDouble();
+
+ if (isHorizontal()) {
+ m_mover->style()->setProperty("width", "", "important",
+ ASSERT_NO_EXCEPTION);
+ } else {
+ m_mover->style()->setProperty("height", "", "important",
+ ASSERT_NO_EXCEPTION);
+ }
+
+ return metrics;
+}
+
+HTMLMarqueeElement::AnimationParameters
+HTMLMarqueeElement::getAnimationParameters() {
+ AnimationParameters parameters;
+ Metrics metrics = getMetrics();
+
+ double totalWidth = metrics.marqueeWidth + metrics.contentWidth;
+ double totalHeight = metrics.marqueeHeight + metrics.contentHeight;
+
+ double innerWidth = metrics.marqueeWidth - metrics.contentWidth;
+ double innerHeight = metrics.marqueeHeight - metrics.contentHeight;
+
+ switch (behavior()) {
+ case Alternate:
+ switch (direction()) {
+ case Right:
+ parameters.transformBegin =
+ createTransform(false, innerWidth >= 0 ? 0 : innerWidth);
+ parameters.transformEnd =
+ createTransform(false, innerWidth >= 0 ? innerWidth : 0);
+ parameters.distance = std::abs(innerWidth);
+ break;
+ case Up:
+ parameters.transformBegin =
+ createTransform(false, innerHeight >= 0 ? innerHeight : 0);
+ parameters.transformEnd =
+ createTransform(false, innerHeight >= 0 ? 0 : innerHeight);
+ parameters.distance = std::abs(innerHeight);
+ break;
+ case Down:
+ parameters.transformBegin =
+ createTransform(false, innerHeight >= 0 ? 0 : innerHeight);
+ parameters.transformEnd =
+ createTransform(false, innerHeight >= 0 ? innerHeight : 0);
+ parameters.distance = std::abs(innerHeight);
+ break;
+ case Left:
+ default:
+ parameters.transformBegin =
+ createTransform(false, innerWidth >= 0 ? innerWidth : 0);
+ parameters.transformEnd =
+ createTransform(false, innerWidth >= 0 ? 0 : innerWidth);
+ parameters.distance = std::abs(innerWidth);
+ }
+
+ if (m_loopCount % 2) {
+ AtomicString transform = parameters.transformBegin;
+ parameters.transformBegin = parameters.transformEnd;
+ parameters.transformEnd = transform;
+ }
+
+ break;
+ case Slide:
+ switch (direction()) {
+ case Right:
+ parameters.transformBegin =
+ createTransform(true, metrics.contentWidth);
+ parameters.transformEnd = createTransform(false, innerWidth);
+ parameters.distance = metrics.marqueeWidth;
+ break;
+ case Up:
+ parameters.transformBegin =
+ createTransform(false, metrics.marqueeHeight);
+ parameters.transformEnd = "translateY(0)";
+ parameters.distance = metrics.marqueeHeight;
+ break;
+ case Down:
+ parameters.transformBegin =
+ createTransform(true, metrics.contentHeight);
+ parameters.transformEnd = createTransform(false, innerHeight);
+ parameters.distance = metrics.marqueeHeight;
+ break;
+ case Left:
+ default:
+ parameters.transformBegin =
+ createTransform(false, metrics.marqueeWidth);
+ parameters.transformEnd = "translateX(0)";
+ parameters.distance = metrics.marqueeWidth;
+ }
+ break;
+ case Scroll:
+ default:
+ switch (direction()) {
+ case Right:
+ parameters.transformBegin =
+ createTransform(true, metrics.contentWidth);
+ parameters.transformEnd =
+ createTransform(false, metrics.marqueeWidth);
+ parameters.distance = totalWidth;
+ break;
+ case Up:
+ parameters.transformBegin =
+ createTransform(false, metrics.marqueeHeight);
+ parameters.transformEnd =
+ createTransform(true, metrics.contentHeight);
+ parameters.distance = totalHeight;
+ break;
+ case Down:
+ parameters.transformBegin =
+ createTransform(true, metrics.contentHeight);
+ parameters.transformEnd =
+ createTransform(false, metrics.marqueeHeight);
+ parameters.distance = totalHeight;
+ break;
+ case Left:
+ default:
+ parameters.transformBegin =
+ createTransform(false, metrics.marqueeWidth);
+ parameters.transformEnd = createTransform(true, metrics.contentWidth);
+ parameters.distance = totalWidth;
+ }
+ break;
+ }
+
+ return parameters;
+}
+
+AtomicString HTMLMarqueeElement::createTransform(bool isNegative,
+ double value) const {
+ char axis = isHorizontal() ? 'X' : 'Y';
+ if (isNegative)
+ return AtomicString(String::format("translate%c(-%fpx)", axis, value));
+ return AtomicString(String::format("translate%c(%fpx)", axis, value));
+}
+
+DEFINE_TRACE(HTMLMarqueeElement) {
+ visitor->trace(m_mover);
+ visitor->trace(m_player);
+ HTMLElement::trace(visitor);
}
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698