Commit 04ce8295 authored by Fredrik Söderquist's avatar Fredrik Söderquist Committed by Commit Bot

Drop instance times (directly) sourced from the attribute when (re)parsing

Since we (now) know which instance times originated as offset-values in
the attribute, we can clear those out before (re)parsing the attribute,
rather than keeping a set of existing times and filter against those.
The only potential downside is that this filtering step is O(n^2).

When doing this we can also remove the HashTraits and associated bits
for hashing SMILTimes. Also start exposing the origin from
SMILTimeWithOrigin directly and change the (few) users.

Adding of the implicit '0s' begin-time is now also done in ParseBeginOrEnd
if needed since it is also treated as having 'attribute' origin.

Bug: 1005185
Change-Id: I3bdf6a099d5853683cbbb516d8778e587353c3bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1818481
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Reviewed-by: default avatarStephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#699430}
parent baac06da
...@@ -152,16 +152,11 @@ class SMILTimeWithOrigin { ...@@ -152,16 +152,11 @@ class SMILTimeWithOrigin {
DISALLOW_NEW(); DISALLOW_NEW();
public: public:
SMILTimeWithOrigin() : origin_(SMILTimeOrigin::kAttribute) {}
SMILTimeWithOrigin(const SMILTime& time, SMILTimeOrigin origin) SMILTimeWithOrigin(const SMILTime& time, SMILTimeOrigin origin)
: time_(time), origin_(origin) {} : time_(time), origin_(origin) {}
const SMILTime& Time() const { return time_; } const SMILTime& Time() const { return time_; }
bool OriginIsScript() const { return origin_ == SMILTimeOrigin::kScript; } SMILTimeOrigin Origin() const { return origin_; }
bool HasSameOrigin(SMILTimeWithOrigin other) const {
return origin_ == other.origin_;
}
private: private:
SMILTime time_; SMILTime time_;
...@@ -207,35 +202,6 @@ inline bool operator!=(const SMILInterval& a, const SMILInterval& b) { ...@@ -207,35 +202,6 @@ inline bool operator!=(const SMILInterval& a, const SMILInterval& b) {
return !(a == b); return !(a == b);
} }
struct SMILTimeHash {
STATIC_ONLY(SMILTimeHash);
static unsigned GetHash(const SMILTime& key) {
return WTF::IntHash<int64_t>::GetHash(key.time_.InMicroseconds());
}
static bool Equal(const SMILTime& a, const SMILTime& b) { return a == b; }
static const bool safe_to_compare_to_empty_or_deleted = true;
};
} // namespace blink } // namespace blink
namespace WTF {
template <>
struct DefaultHash<blink::SMILTime> {
typedef blink::SMILTimeHash Hash;
};
template <>
struct HashTraits<blink::SMILTime> : GenericHashTraits<blink::SMILTime> {
static blink::SMILTime EmptyValue() { return blink::SMILTime::Unresolved(); }
static void ConstructDeletedValue(blink::SMILTime& slot, bool) {
slot = blink::SMILTime::Earliest();
}
static bool IsDeletedValue(blink::SMILTime value) {
return value == blink::SMILTime::Earliest();
}
};
} // namespace WTF
#endif // THIRD_PARTY_BLINK_RENDERER_CORE_SVG_ANIMATION_SMIL_TIME_H_ #endif // THIRD_PARTY_BLINK_RENDERER_CORE_SVG_ANIMATION_SMIL_TIME_H_
...@@ -268,10 +268,11 @@ void SVGSMILElement::BuildPendingResource() { ...@@ -268,10 +268,11 @@ void SVGSMILElement::BuildPendingResource() {
ConnectConditions(); ConnectConditions();
} }
static inline void ClearTimesWithDynamicOrigins( static inline void RemoveInstanceTimesWithOrigin(
Vector<SMILTimeWithOrigin>& time_list) { Vector<SMILTimeWithOrigin>& time_list,
SMILTimeOrigin origin) {
for (int i = time_list.size() - 1; i >= 0; --i) { for (int i = time_list.size() - 1; i >= 0; --i) {
if (time_list[i].OriginIsScript()) if (time_list[i].Origin() == origin)
time_list.EraseAt(i); time_list.EraseAt(i);
} }
} }
...@@ -309,8 +310,10 @@ Node::InsertionNotificationRequest SVGSMILElement::InsertedInto( ...@@ -309,8 +310,10 @@ Node::InsertionNotificationRequest SVGSMILElement::InsertedInto(
// "If no attribute is present, the default begin value (an offset-value of 0) // "If no attribute is present, the default begin value (an offset-value of 0)
// must be evaluated." // must be evaluated."
if (!FastHasAttribute(svg_names::kBeginAttr)) if (!FastHasAttribute(svg_names::kBeginAttr) && begin_times_.IsEmpty()) {
begin_times_.push_back(SMILTimeWithOrigin()); begin_times_.push_back(
SMILTimeWithOrigin(SMILTime(), SMILTimeOrigin::kAttribute));
}
if (is_waiting_for_first_interval_) if (is_waiting_for_first_interval_)
ResolveFirstInterval(); ResolveFirstInterval();
...@@ -475,22 +478,29 @@ void SVGSMILElement::ParseBeginOrEnd(const String& parse_string, ...@@ -475,22 +478,29 @@ void SVGSMILElement::ParseBeginOrEnd(const String& parse_string,
begin_or_end == kBegin ? begin_times_ : end_times_; begin_or_end == kBegin ? begin_times_ : end_times_;
if (begin_or_end == kEnd) if (begin_or_end == kEnd)
has_end_event_conditions_ = false; has_end_event_conditions_ = false;
HashSet<SMILTime> existing;
for (const auto& instance_time : time_list) { // Remove any previously added offset-values.
if (!instance_time.Time().IsUnresolved()) // TODO(fs): Ought to remove instance times originating from sync-bases,
existing.insert(instance_time.Time()); // events etc. as well if those conditions are no longer in the attribute.
} RemoveInstanceTimesWithOrigin(time_list, SMILTimeOrigin::kAttribute);
Vector<String> split_string; Vector<String> split_string;
parse_string.Split(';', split_string); parse_string.Split(';', split_string);
for (const auto& item : split_string) { for (const auto& item : split_string) {
SMILTime value = ParseClockValue(item); SMILTime value = ParseClockValue(item);
if (value.IsUnresolved()) { if (value.IsUnresolved()) {
ParseCondition(item, begin_or_end); ParseCondition(item, begin_or_end);
} else if (!existing.Contains(value)) { } else {
time_list.push_back( time_list.push_back(
SMILTimeWithOrigin(value, SMILTimeOrigin::kAttribute)); SMILTimeWithOrigin(value, SMILTimeOrigin::kAttribute));
} }
} }
// "If no attribute is present, the default begin value (an offset-value of 0)
// must be evaluated."
if (begin_or_end == kBegin && parse_string.IsNull()) {
begin_times_.push_back(
SMILTimeWithOrigin(SMILTime(), SMILTimeOrigin::kAttribute));
}
std::sort(time_list.begin(), time_list.end()); std::sort(time_list.begin(), time_list.end());
} }
...@@ -705,7 +715,7 @@ static void InsertSortedAndUnique(Vector<SMILTimeWithOrigin>& list, ...@@ -705,7 +715,7 @@ static void InsertSortedAndUnique(Vector<SMILTimeWithOrigin>& list,
break; break;
// If they share both time and origin, we don't need to add it, // If they share both time and origin, we don't need to add it,
// we just need to react. // we just need to react.
if (position->HasSameOrigin(time)) if (position->Origin() == time.Origin())
return; return;
} }
list.insert(position - list.begin(), time); list.insert(position - list.begin(), time);
...@@ -717,15 +727,14 @@ void SVGSMILElement::AddInstanceTime(BeginOrEnd begin_or_end, ...@@ -717,15 +727,14 @@ void SVGSMILElement::AddInstanceTime(BeginOrEnd begin_or_end,
SMILTime current_presentation_time = SMILTime current_presentation_time =
time_container_ ? time_container_->CurrentDocumentTime() : SMILTime(); time_container_ ? time_container_->CurrentDocumentTime() : SMILTime();
DCHECK(!current_presentation_time.IsUnresolved()); DCHECK(!current_presentation_time.IsUnresolved());
SMILTimeWithOrigin time_with_origin(time, origin);
// Ignore new instance times for 'end' if the element is not active // Ignore new instance times for 'end' if the element is not active
// and the origin is script. // and the origin is script.
if (begin_or_end == kEnd && GetActiveState() == kInactive && if (begin_or_end == kEnd && GetActiveState() == kInactive &&
time_with_origin.OriginIsScript()) origin == SMILTimeOrigin::kScript)
return; return;
Vector<SMILTimeWithOrigin>& list = Vector<SMILTimeWithOrigin>& list =
begin_or_end == kBegin ? begin_times_ : end_times_; begin_or_end == kBegin ? begin_times_ : end_times_;
InsertSortedAndUnique(list, time_with_origin); InsertSortedAndUnique(list, SMILTimeWithOrigin(time, origin));
if (begin_or_end == kBegin) if (begin_or_end == kBegin)
BeginListChanged(current_presentation_time); BeginListChanged(current_presentation_time);
else else
...@@ -1237,8 +1246,8 @@ void SVGSMILElement::BeginByLinkActivation() { ...@@ -1237,8 +1246,8 @@ void SVGSMILElement::BeginByLinkActivation() {
} }
void SVGSMILElement::EndedActiveInterval() { void SVGSMILElement::EndedActiveInterval() {
ClearTimesWithDynamicOrigins(begin_times_); RemoveInstanceTimesWithOrigin(begin_times_, SMILTimeOrigin::kScript);
ClearTimesWithDynamicOrigins(end_times_); RemoveInstanceTimesWithOrigin(end_times_, SMILTimeOrigin::kScript);
} }
void SVGSMILElement::ScheduleRepeatEvents() { void SVGSMILElement::ScheduleRepeatEvents() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment