Commit 82088ec9 authored by Keishi Hattori's avatar Keishi Hattori Committed by Commit Bot

Revert "Change SMILTimeContainer::scheduled_animations_ to use ephemeron"

This reverts commit bea6537a.

Reason for revert: clusterfuzz found many issues
crbug.com/874396
crbug.com/874420
crbug.com/874431
crbug.com/874425
crbug.com/874430
crbug.com/874429
crbug.com/874423
crbug.com/874404

Original change's description:
> Change SMILTimeContainer::scheduled_animations_ to use ephemeron
> 
> WeakMember as part of a HashMap entry can cause unexpected clearing when incremental marking is enabled.
> 
> This CL changes SMILTimeContainer::scheduled_animations_ to use a epehemeron instead.
> 
> Bug: 870306
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
> Change-Id: I8b653825de0bc683a5151576385716141c78c454
> Reviewed-on: https://chromium-review.googlesource.com/1166753
> Commit-Queue: Keishi Hattori <keishi@chromium.org>
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#583156}

TBR=pdr@chromium.org,haraken@chromium.org,keishi@chromium.org,mlippautz@chromium.org

Change-Id: Id727b6da2d0a42341bde5d9f6c2c9f32a43d447e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 870306
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1175881Reviewed-by: default avatarKeishi Hattori <keishi@chromium.org>
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583247}
parent b8b5e2f2
...@@ -79,14 +79,13 @@ void SMILTimeContainer::Schedule(SVGSMILElement* animation, ...@@ -79,14 +79,13 @@ void SMILTimeContainer::Schedule(SVGSMILElement* animation,
DCHECK(!prevent_scheduled_animations_changes_); DCHECK(!prevent_scheduled_animations_changes_);
#endif #endif
AttributeAnimationsMap& attribute_map = ElementAttributePair key(target, attribute_name);
scheduled_animations_.insert(target, AttributeAnimationsMap()) Member<AnimationsLinkedHashSet>& scheduled =
.stored_value->value; scheduled_animations_.insert(key, nullptr).stored_value->value;
AnimationsLinkedHashSet& scheduled = if (!scheduled)
attribute_map.insert(attribute_name, AnimationsLinkedHashSet()) scheduled = new AnimationsLinkedHashSet;
.stored_value->value; DCHECK(!scheduled->Contains(animation));
DCHECK(!scheduled.Contains(animation)); scheduled->insert(animation);
scheduled.insert(animation);
SMILTime next_fire_time = animation->NextProgressTime(); SMILTime next_fire_time = animation->NextProgressTime();
if (next_fire_time.IsFinite()) if (next_fire_time.IsFinite())
...@@ -102,20 +101,16 @@ void SMILTimeContainer::Unschedule(SVGSMILElement* animation, ...@@ -102,20 +101,16 @@ void SMILTimeContainer::Unschedule(SVGSMILElement* animation,
DCHECK(!prevent_scheduled_animations_changes_); DCHECK(!prevent_scheduled_animations_changes_);
#endif #endif
GroupedAnimationsMap::iterator it = scheduled_animations_.find(target); ElementAttributePair key(target, attribute_name);
CHECK(it != scheduled_animations_.end()); GroupedAnimationsMap::iterator it = scheduled_animations_.find(key);
AttributeAnimationsMap& attribute_map = it->value; DCHECK_NE(it, scheduled_animations_.end());
AttributeAnimationsMap::iterator attribute_map_it = AnimationsLinkedHashSet* scheduled = it->value.Get();
attribute_map.find(attribute_name); DCHECK(scheduled);
DCHECK(attribute_map_it != attribute_map.end()); AnimationsLinkedHashSet::iterator it_animation = scheduled->find(animation);
AnimationsLinkedHashSet& scheduled = attribute_map_it->value; DCHECK(it_animation != scheduled->end());
AnimationsLinkedHashSet::iterator it_animation = scheduled.find(animation); scheduled->erase(it_animation);
DCHECK(it_animation != scheduled.end());
scheduled.erase(it_animation); if (scheduled->IsEmpty())
if (scheduled.IsEmpty())
attribute_map.erase(attribute_map_it);
if (attribute_map.IsEmpty())
scheduled_animations_.erase(it); scheduled_animations_.erase(it);
} }
...@@ -234,13 +229,13 @@ void SMILTimeContainer::SetElapsed(double elapsed) { ...@@ -234,13 +229,13 @@ void SMILTimeContainer::SetElapsed(double elapsed) {
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
prevent_scheduled_animations_changes_ = true; prevent_scheduled_animations_changes_ = true;
#endif #endif
for (const auto& attribute_entry : scheduled_animations_) { for (const auto& entry : scheduled_animations_) {
for (const auto& entry : attribute_entry.value) { if (!entry.key.first)
const AnimationsLinkedHashSet& scheduled = entry.value; continue;
for (SVGSMILElement* element : scheduled) {
element->Reset(); AnimationsLinkedHashSet* scheduled = entry.value.Get();
} for (SVGSMILElement* element : *scheduled)
} element->Reset();
} }
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
prevent_scheduled_animations_changes_ = false; prevent_scheduled_animations_changes_ = false;
...@@ -433,65 +428,62 @@ SMILTime SMILTimeContainer::UpdateAnimations(double elapsed, ...@@ -433,65 +428,62 @@ SMILTime SMILTimeContainer::UpdateAnimations(double elapsed,
if (document_order_indexes_dirty_) if (document_order_indexes_dirty_)
UpdateDocumentOrderIndexes(); UpdateDocumentOrderIndexes();
HeapHashSet<ElementAttributePair> invalid_keys;
using AnimationsVector = HeapVector<Member<SVGSMILElement>>; using AnimationsVector = HeapVector<Member<SVGSMILElement>>;
AnimationsVector animations_to_apply; AnimationsVector animations_to_apply;
AnimationsVector scheduled_animations_in_same_group; AnimationsVector scheduled_animations_in_same_group;
for (auto attribute_entry : scheduled_animations_) { for (const auto& entry : scheduled_animations_) {
AttributeAnimationsMap attribute_map = attribute_entry.value; if (!entry.key.first || entry.value->IsEmpty()) {
Vector<QualifiedName> invalid_keys; invalid_keys.insert(entry.key);
for (const auto& entry : attribute_map) { continue;
if (entry.value.IsEmpty()) { }
invalid_keys.push_back(entry.key);
continue;
}
// Sort according to priority. Elements with later begin time have higher // Sort according to priority. Elements with later begin time have higher
// priority. In case of a tie, document order decides. // priority. In case of a tie, document order decides.
// FIXME: This should also consider timing relationships between the // FIXME: This should also consider timing relationships between the
// elements. Dependents have higher priority. // elements. Dependents have higher priority.
CopyToVector(entry.value, scheduled_animations_in_same_group); CopyToVector(*entry.value, scheduled_animations_in_same_group);
std::sort(scheduled_animations_in_same_group.begin(), std::sort(scheduled_animations_in_same_group.begin(),
scheduled_animations_in_same_group.end(), scheduled_animations_in_same_group.end(),
PriorityCompare(elapsed)); PriorityCompare(elapsed));
AnimationsVector sandwich; AnimationsVector sandwich;
for (const auto& it_animation : scheduled_animations_in_same_group) { for (const auto& it_animation : scheduled_animations_in_same_group) {
SVGSMILElement* animation = it_animation.Get(); SVGSMILElement* animation = it_animation.Get();
DCHECK_EQ(animation->TimeContainer(), this); DCHECK_EQ(animation->TimeContainer(), this);
DCHECK(animation->HasValidTarget()); DCHECK(animation->HasValidTarget());
// This will calculate the contribution from the animation and update // This will calculate the contribution from the animation and update
// timing. // timing.
if (animation->Progress(elapsed, seek_to_time)) { if (animation->Progress(elapsed, seek_to_time)) {
sandwich.push_back(animation); sandwich.push_back(animation);
} else { } else {
animation->ClearAnimatedType(); animation->ClearAnimatedType();
}
SMILTime next_fire_time = animation->NextProgressTime();
if (next_fire_time.IsFinite())
earliest_fire_time = std::min(next_fire_time, earliest_fire_time);
} }
if (!sandwich.IsEmpty()) { SMILTime next_fire_time = animation->NextProgressTime();
// Results are accumulated to the first animation that animates and if (next_fire_time.IsFinite())
// contributes to a particular element/attribute pair. earliest_fire_time = std::min(next_fire_time, earliest_fire_time);
// Only reset the animated type to the base value once for }
// the lowest priority animation that animates and
// contributes to a particular element/attribute pair. if (!sandwich.IsEmpty()) {
SVGSMILElement* result_element = sandwich.front(); // Results are accumulated to the first animation that animates and
result_element->ResetAnimatedType(); // contributes to a particular element/attribute pair.
// Only reset the animated type to the base value once for
// Go through the sandwich from lowest prio to highest and generate // the lowest priority animation that animates and
// the animated value (if any.) // contributes to a particular element/attribute pair.
for (const auto& animation : sandwich) SVGSMILElement* result_element = sandwich.front();
animation->UpdateAnimatedValue(result_element); result_element->ResetAnimatedType();
animations_to_apply.push_back(result_element); // Go through the sandwich from lowest prio to highest and generate
} // the animated value (if any.)
for (const auto& animation : sandwich)
animation->UpdateAnimatedValue(result_element);
animations_to_apply.push_back(result_element);
} }
attribute_map.RemoveAll(invalid_keys);
} }
scheduled_animations_.RemoveAll(invalid_keys);
if (animations_to_apply.IsEmpty()) { if (animations_to_apply.IsEmpty()) {
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
......
...@@ -135,11 +135,10 @@ class SMILTimeContainer : public GarbageCollectedFinalized<SMILTimeContainer> { ...@@ -135,11 +135,10 @@ class SMILTimeContainer : public GarbageCollectedFinalized<SMILTimeContainer> {
TaskRunnerTimer<SMILTimeContainer> wakeup_timer_; TaskRunnerTimer<SMILTimeContainer> wakeup_timer_;
TaskRunnerTimer<SMILTimeContainer> animation_policy_once_timer_; TaskRunnerTimer<SMILTimeContainer> animation_policy_once_timer_;
using ElementAttributePair = std::pair<WeakMember<SVGElement>, QualifiedName>;
using AnimationsLinkedHashSet = HeapLinkedHashSet<WeakMember<SVGSMILElement>>; using AnimationsLinkedHashSet = HeapLinkedHashSet<WeakMember<SVGSMILElement>>;
using AttributeAnimationsMap =
HeapHashMap<QualifiedName, AnimationsLinkedHashSet>;
using GroupedAnimationsMap = using GroupedAnimationsMap =
HeapHashMap<WeakMember<SVGElement>, AttributeAnimationsMap>; HeapHashMap<ElementAttributePair, Member<AnimationsLinkedHashSet>>;
GroupedAnimationsMap scheduled_animations_; GroupedAnimationsMap scheduled_animations_;
Member<SVGSVGElement> owner_svg_element_; Member<SVGSVGElement> owner_svg_element_;
......
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