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

replayio · Jul 2, 2023 · 0384e14 · 0384e14
1 parent fd4f41b
commit 0384e14
Show file tree

Hide file tree

Showing 14 changed files with 53 additions and 5 deletions.
diff --git a/third_party/blink/renderer/core/dom/element_rare_data.h b/third_party/blink/renderer/core/dom/element_rare_data.h
@@ -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(); }

diff --git a/third_party/blink/renderer/core/dom/id_target_observer.cc b/third_party/blink/renderer/core/dom/id_target_observer.cc
@@ -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);
 }
 
@@ -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
diff --git a/third_party/blink/renderer/core/dom/id_target_observer.h b/third_party/blink/renderer/core/dom/id_target_observer.h
@@ -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);
 

diff --git a/third_party/blink/renderer/core/dom/id_target_observer_registry.cc b/third_party/blink/renderer/core/dom/id_target_observer_registry.cc
@@ -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();
   }

diff --git a/third_party/blink/renderer/core/layout/svg/layout_svg_inline_text.cc b/third_party/blink/renderer/core/layout/svg/layout_svg_inline_text.cc
@@ -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());
 }

diff --git a/third_party/blink/renderer/core/svg/svg_element.cc b/third_party/blink/renderer/core/svg/svg_element.cc
@@ -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) {
@@ -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() {
@@ -1120,6 +1126,9 @@ void SVGElement::InvalidateInstances() {
   }
 
   SvgRareData()->ElementInstances().clear();
+
+  if (recordreplay::IsRecordingOrReplaying("avoid-weak-pointers", "SVGElement"))
+    EnsureSVGRareData()->ReplayStrongElementInstances().clear();
 }
 
 void SVGElement::SetNeedsStyleRecalcForInstances(

diff --git a/third_party/blink/renderer/core/svg/svg_element_rare_data.cc b/third_party/blink/renderer/core/svg/svg_element_rare_data.cc
@@ -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() {

diff --git a/third_party/blink/renderer/core/svg/svg_element_rare_data.h b/third_party/blink/renderer/core/svg/svg_element_rare_data.h
@@ -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;
@@ -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_;

diff --git a/third_party/blink/renderer/core/svg/svg_use_element.cc b/third_party/blink/renderer/core/svg/svg_use_element.cc
@@ -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();

diff --git a/third_party/blink/renderer/platform/bindings/script_wrappable.h b/third_party/blink/renderer/platform/bindings/script_wrappable.h
@@ -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");

diff --git a/third_party/blink/renderer/platform/fonts/font.cc b/third_party/blink/renderer/platform/fonts/font.cc
@@ -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();

diff --git a/third_party/blink/renderer/platform/fonts/font_fallback_map.cc b/third_party/blink/renderer/platform/fonts/font_fallback_map.cc
@@ -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();
 }
 

diff --git a/third_party/blink/renderer/platform/fonts/font_selector.cc b/third_party/blink/renderer/platform/fonts/font_selector.cc
@@ -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_);

diff --git a/third_party/blink/renderer/platform/fonts/font_selector.h b/third_party/blink/renderer/platform/fonts/font_selector.h
@@ -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&,
@@ -154,6 +160,7 @@ class PLATFORM_EXPORT FontSelector : public FontCacheClient {
 
  private:
   Member<FontFallbackMap> font_fallback_map_;
+  int record_replay_id_ = 0;
 };
 
 }  // namespace blink