Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUN-2286: More Asserts and fixes for 1436 #788

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions third_party/blink/renderer/core/dom/element_rare_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ class ElementRareData final : public NodeRareData {
void SetShadowRoot(ShadowRoot& shadow_root) {
DCHECK(!shadow_root_);
shadow_root_ = &shadow_root;
recordreplay::Assert("[RUN-1436-2226] ElementRareData::SetShadowRoot %d",
shadow_root.RecordReplayId());
}

NamedNodeMap* AttributeMap() const { return attribute_map_.Get(); }
Expand Down
7 changes: 7 additions & 0 deletions third_party/blink/renderer/core/dom/id_target_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ namespace blink {
IdTargetObserver::IdTargetObserver(IdTargetObserverRegistry& observer_registry,
const AtomicString& id)
: registry_(&observer_registry), id_(id) {
recordreplay::Assert("[RUN-1436-2286] IdTargetObserver %s", id.Utf8().c_str());
Registry().AddObserver(id_, this);
}

Expand All @@ -45,4 +46,10 @@ void IdTargetObserver::Unregister() {
Registry().RemoveObserver(id_, this);
}

unsigned IdTargetObserver::GetHash() const {
// WTF::StringHash comments: "The GetHash() functions on StringHash do
// not support null strings".
return id_.IsNull() ? 0 : WTF::StringHash::GetHash(id_);
}

} // namespace blink
4 changes: 4 additions & 0 deletions third_party/blink/renderer/core/dom/id_target_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ class IdTargetObserver : public GarbageCollected<IdTargetObserver> {
virtual void IdTargetChanged() = 0;
virtual void Unregister();

// Avoid pointer-based hash by computing hash based on id_ instead.
// TODO: [RUN-1741] Remove this.
unsigned GetHash() const;

protected:
IdTargetObserver(IdTargetObserverRegistry&, const AtomicString& id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ void IdTargetObserverRegistry::NotifyObserversInternal(const AtomicString& id) {

HeapVector<Member<IdTargetObserver>> copy(*notifying_observers_in_set_);
for (const auto& observer : copy) {
recordreplay::Assert(
"[RUN-1436-2286] IdTargetObserverRegistry::NotifyObserversInternal %d %u",
notifying_observers_in_set_->Contains(observer), observer->GetHash());
if (notifying_observers_in_set_->Contains(observer))
observer->IdTargetChanged();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,12 @@ void LayoutSVGInlineText::ComputeNewScaledFontForStyle(
font_description.SetWordSpacing(font_description.WordSpacing() *
scaling_factor / zoom);

recordreplay::Assert(
"[RUN-1436-2286] LayoutSVGInlineText::ComputeNewScaledFontForStyle %d",
document.GetStyleEngine().GetFontSelector()
? document.GetStyleEngine().GetFontSelector()->RecordReplayId()
: -1);

scaled_font =
Font(font_description, document.GetStyleEngine().GetFontSelector());
}
Expand Down
9 changes: 9 additions & 0 deletions third_party/blink/renderer/core/svg/svg_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,9 @@ void SVGElement::AddInstance(SVGElement* instance) {
DCHECK(!instances.Contains(instance));

instances.insert(instance);
if (recordreplay::IsRecordingOrReplaying("avoid-weak-pointers",
"SVGElement"))
EnsureSVGRareData()->ReplayStrongElementInstances().insert(instance);
}

void SVGElement::RemoveInstance(SVGElement* instance) {
Expand All @@ -611,6 +614,9 @@ void SVGElement::RemoveInstance(SVGElement* instance) {
SvgRareData()->ElementInstances();

instances.erase(instance);

if (recordreplay::IsRecordingOrReplaying("avoid-weak-pointers", "SVGElement"))
EnsureSVGRareData()->ReplayStrongElementInstances().erase(instance);
}

static HeapHashSet<WeakMember<SVGElement>>& EmptyInstances() {
Expand Down Expand Up @@ -1120,6 +1126,9 @@ void SVGElement::InvalidateInstances() {
}

SvgRareData()->ElementInstances().clear();

if (recordreplay::IsRecordingOrReplaying("avoid-weak-pointers", "SVGElement"))
EnsureSVGRareData()->ReplayStrongElementInstances().clear();
}

void SVGElement::SetNeedsStyleRecalcForInstances(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ void SVGElementRareData::Trace(Visitor* visitor) const {
visitor->Trace(corresponding_element_);
visitor->Trace(resource_client_);
visitor->Trace(smil_animations_);
visitor->Trace(replay_strong_element_instances_);
}

AffineTransform* SVGElementRareData::AnimateMotionTransform() {
Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/svg/svg_element_rare_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ class SVGElementRareData final : public GarbageCollected<SVGElementRareData> {
return element_instances_;
}

HeapHashSet<Member<SVGElement>>& ReplayStrongElementInstances() {
return replay_strong_element_instances_;
}

bool InstanceUpdatesBlocked() const { return instances_updates_blocked_; }
void SetInstanceUpdatesBlocked(bool value) {
instances_updates_blocked_ = value;
Expand Down Expand Up @@ -107,6 +111,7 @@ class SVGElementRareData final : public GarbageCollected<SVGElementRareData> {
SVGElementSet outgoing_references_;
SVGElementSet incoming_references_;
HeapHashSet<WeakMember<SVGElement>> element_instances_;
HeapHashSet<Member<SVGElement>> replay_strong_element_instances_;
Member<SVGElement> corresponding_element_;
Member<SVGElementResourceClient> resource_client_;
Member<ElementSMILAnimations> smil_animations_;
Expand Down
3 changes: 3 additions & 0 deletions third_party/blink/renderer/core/svg/svg_use_element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,9 @@ bool SVGUseElement::ShadowTreeRebuildPending() const {
}

void SVGUseElement::InvalidateShadowTree() {
recordreplay::Assert(
"[RUN-1436-2286] SVGUseElement::InvalidateTargetReference A %d %d",
RecordReplayId(), ShadowTreeRebuildPending());
if (ShadowTreeRebuildPending())
return;
ScheduleShadowTreeRecreation();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ class PLATFORM_EXPORT ScriptWrappable

int RecordReplayId() const { return record_replay_id_; }

// Avoid pointer-based hashes for ScriptWrappable.
// TODO: [RUN-1741] Remove this.
unsigned GetHash() const { return (unsigned)record_replay_id_; }

protected:
ScriptWrappable() {
record_replay_id_ = recordreplay::NewIdAnyThread("ScriptWrappable");
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/platform/fonts/font.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ namespace blink {
namespace {

FontFallbackMap& GetFontFallbackMap(FontSelector* font_selector) {
recordreplay::Assert("[RUN-1436-2286] GetFontFallbackMap %d",
font_selector ? font_selector->RecordReplayId() : -1);
if (font_selector)
return font_selector->GetFontFallbackMap();
return FontCache::Get().GetFontFallbackMap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,13 @@ void FontFallbackMap::Remove(const FontDescription& font_description) {
DCHECK_NE(iter, fallback_list_for_description_.end());
DCHECK(iter->value->IsValid());
DCHECK(iter->value->HasOneRef());
recordreplay::Assert("[RUN-1436-2260] FontFallbackMap::Remove %s",
font_description.ToString().Utf8().c_str());
fallback_list_for_description_.erase(iter);
}

void FontFallbackMap::InvalidateAll() {
lock_.AssertAcquired();
for (auto& entry : fallback_list_for_description_)
entry.value->MarkInvalid();
recordreplay::Assert("[RUN-1436-2260] FontFallbackMap::InvalidateAll");
fallback_list_for_description_.clear();
}

Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/platform/fonts/font_selector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ void FontSelector::Trace(Visitor* visitor) const {
}

FontFallbackMap& FontSelector::GetFontFallbackMap() {
recordreplay::Assert("[RUN-1436-2286] FontSelector::GetFontFallbackMap %d %d",
record_replay_id_, !!font_fallback_map_);
if (!font_fallback_map_) {
font_fallback_map_ = MakeGarbageCollected<FontFallbackMap>(this);
RegisterForInvalidationCallbacks(font_fallback_map_);
Expand Down
7 changes: 7 additions & 0 deletions third_party/blink/renderer/platform/fonts/font_selector.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,13 @@ class PLATFORM_EXPORT FontSelector : public FontCacheClient {

void Trace(Visitor* visitor) const override;

int RecordReplayId() const { return record_replay_id_; }

protected:
FontSelector() {
record_replay_id_ = recordreplay::NewIdAnyThread("FontSelector");
}

static AtomicString FamilyNameFromSettings(
const GenericFontFamilySettings&,
const FontDescription&,
Expand All @@ -154,6 +160,7 @@ class PLATFORM_EXPORT FontSelector : public FontCacheClient {

private:
Member<FontFallbackMap> font_fallback_map_;
int record_replay_id_ = 0;
};

} // namespace blink
Expand Down