Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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: 1 addition & 1 deletion DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ vars = {
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling V8
# and whatever else without interference from each other.
'v8_revision': '1a0e6a82c5f5cb43513b781e027569896b60bcb4',
'v8_revision': '4902428d14d7b4244afbbdae1fbfdfa282eeb5b7',
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling swarming_client
# and whatever else without interference from each other.
Expand Down
2 changes: 1 addition & 1 deletion REPLAY_BACKEND_REV
Original file line number Diff line number Diff line change
@@ -1 +1 @@
6c7e9991dcf557721c0566c2c0d68ac94f2a5b10
8d5f91e301c3d248654f3158db03fd9098eca8bb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ class MojoPageTimingSender : public PageTimingSender {
const absl::optional<blink::MobileFriendliness>& mobile_friendliness,
uint32_t soft_navigation_count) override {
DCHECK(page_load_metrics_);
mojo::internal::AutoRecordReplayAssertBufferAllocations assertsEnabled("TT-366-1124");
page_load_metrics_->UpdateTiming(
limited_sending_mode_ ? CreatePageLoadTiming() : timing->Clone(),
metadata->Clone(), new_features, std::move(resources),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,16 @@ bool UseCounterFeatureTracker::Test(const UseCounterFeature& feature) const {

bool UseCounterFeatureTracker::TestAndSet(const UseCounterFeature& feature) {
bool has_record = Test(feature);
REPLAY_ASSERT("[TT-366-1456] UseCounterFeatureTracker::TestAndSet %d %d %u",
has_record,
feature.type(),
feature.value());
REPLAY_ASSERT_IF(
// [Rebase-Check] This might change with a rebase.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what exactly might change? can the answer go in the comment?

// Ignore Cross-origin access check (WebFeature::kCrossOriginPropertyAccess)
// because it looks like an allowed divergence.
(int)feature.type() != 0 || ((int)feature.value() != 1977 &&
((int)feature.value() < 4115 || (int)feature.value() > 4144)),
"[TT-366-1480] UseCounterFeatureTracker::TestAndSet %d %d %u",
has_record,
feature.type(),
feature.value());
Set(feature, true);
return has_record;
}
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/css/css_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ class CORE_EXPORT CSSValue : public GarbageCollected<CSSValue> {
void TraceAfterDispatch(blink::Visitor* visitor) const {}
void Trace(Visitor*) const;

uint8_t ReplayGetClassType() const { return class_type_; }

protected:
enum ClassType {
kNumericLiteralClass,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,6 @@ KURL CSSParserContext::CompleteURL(const String& url) const {
}

void CSSParserContext::Count(WebFeature feature) const {
REPLAY_ASSERT("[TT-366-1467] CSSParserContext::Count feat %d %d",
IsUseCounterRecordingEnabled(),
feature);
if (IsUseCounterRecordingEnabled())
document_->CountUse(feature);
}
Expand All @@ -242,7 +239,7 @@ void CSSParserContext::CountDeprecation(WebFeature feature) const {
}

void CSSParserContext::Count(CSSParserMode mode, CSSPropertyID property) const {
REPLAY_ASSERT("[TT-366-1467] CSSParserContext::Count prop %d %d %d",
REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1480] CSSParserContext::Count prop %d %d %d",
IsUseCounterRecordingEnabled(),
(int)mode,
property);
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/css/parser/css_parser_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,8 @@ StyleRuleBase* CSSParserImpl::ConsumeAtRule(CSSParserTokenStream& stream,
StyleRuleBase* CSSParserImpl::ConsumeQualifiedRule(
CSSParserTokenStream& stream,
AllowedRulesType allowed_rules) {
REPLAY_ASSERT("[TT-366-1480] CSSParserImpl::ConsumeQualifiedRule %d",
allowed_rules);
if (allowed_rules <= kRegularRules) {
return ConsumeStyleRule(stream);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,9 @@ ArenaUniquePtr<CSSParserSelector> CSSSelectorParser::ConsumeRelativeSelector(
ArenaUniquePtr<CSSParserSelector> CSSSelectorParser::ConsumeComplexSelector(
CSSParserTokenRange& range) {
ArenaUniquePtr<CSSParserSelector> selector = ConsumeCompoundSelector(range);
REPLAY_ASSERT("[TT-366-1480] CSSSelectorParser::ConsumeComplexSelector %u %d",
range.size(),
!!selector);
if (!selector)
return nullptr;

Expand Down
4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/css/resolver/font_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ void FontBuilder::CreateFont(ComputedStyle& style,
const ComputedStyle* parent_style) {
DCHECK(document_);

recordreplay::Assert("[RUN-1436-2237] FontBuilder::CreateFont A %u", flags_);
REPLAY_ASSERT("[TT-366-1480] FontBuilder::CreateFont A %u", flags_);
if (!flags_)
return;

Expand All @@ -496,7 +496,7 @@ void FontBuilder::CreateFont(ComputedStyle& style,
FontSelector* font_selector = ComputeFontSelector(style);
UpdateAdjustedSize(description, style, font_selector);

recordreplay::Assert("[RUN-1436-2237] FontBuilder::CreateFont B %d",
REPLAY_ASSERT("[TT-366-1480] FontBuilder::CreateFont B %d",
!!font_selector);
style.SetFontInternal(Font(description, font_selector));
flags_ = 0;
Expand Down
11 changes: 6 additions & 5 deletions third_party/blink/renderer/core/css/resolver/match_result.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,33 +68,34 @@ void MatchResult::AddMatchedProperties(
void MatchResult::FinishAddingUARules() {
DCHECK_EQ(current_origin_, CascadeOrigin::kUserAgent);
current_origin_ = CascadeOrigin::kUser;
REPLAY_ASSERT("[RUN-2424-3053] FinishAddingUARules");
REPLAY_ASSERT("[TT-366-1480] FinishAddingUARules");
}

void MatchResult::FinishAddingUserRules() {
DCHECK_EQ(current_origin_, CascadeOrigin::kUser);
current_origin_ = CascadeOrigin::kAuthorPresentationalHint;
REPLAY_ASSERT("[RUN-2424-3053] FinishAddingUserRules");
REPLAY_ASSERT("[TT-366-1480] FinishAddingUserRules");
}

void MatchResult::FinishAddingPresentationalHints() {
DCHECK_EQ(current_origin_, CascadeOrigin::kAuthorPresentationalHint);
current_origin_ = CascadeOrigin::kAuthor;
REPLAY_ASSERT("[RUN-2424-3053] FinishAddingPresentationalHints");
REPLAY_ASSERT("[TT-366-1480] FinishAddingPresentationalHints");
}

void MatchResult::FinishAddingAuthorRulesForTreeScope(
const TreeScope& tree_scope) {
DCHECK_EQ(current_origin_, CascadeOrigin::kAuthor);
tree_scopes_.push_back(&tree_scope);
current_tree_order_ = base::ClampAdd(current_tree_order_, 1);
REPLAY_ASSERT("[RUN-2424-3053] MatchResult::FinishAddingAuthorRulesForTreeScope %u",
REPLAY_ASSERT("[TT-366-1480] MatchResult::FinishAddingAuthorRulesForTreeScope %d %u",
tree_scope.RecordReplayId(),
tree_scopes_.size()
);
}

void MatchResult::Reset() {
REPLAY_ASSERT("[RUN-2424-3053] MatchResult::Reset %u",
REPLAY_ASSERT("[TT-366-1480] MatchResult::Reset %u",
(unsigned)current_tree_order_
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ FontDescription::FamilyDescription StyleBuilderConverterBase::ConvertFontFamily(
const CSSValue& value,
FontBuilder* font_builder,
const Document* document_for_count) {
REPLAY_ASSERT("[TT-366-1480] StyleBuilderConverterBase::ConvertFontFamily %s",
value.CssText().Utf8().c_str());
FontDescription::FamilyDescription desc(FontDescription::kNoFamily);

if (const auto* system_font =
Expand Down
18 changes: 18 additions & 0 deletions third_party/blink/renderer/core/css/resolver/style_cascade.cc
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ void StyleCascade::AddInterpolations(const ActiveInterpolationsMap* map,
}

void StyleCascade::Apply(CascadeFilter filter) {
REPLAY_ASSERT("[TT-366-1480] StyleCascade::Apply %d", (int)generation_);
AnalyzeIfNeeded();

CascadeResolver resolver(filter, ++generation_);
Expand Down Expand Up @@ -637,6 +638,12 @@ void StyleCascade::LookupAndApply(const CSSProperty& property,
DCHECK(!resolver.IsLocked(property));

CascadePriority* priority = map_.Find(name);

REPLAY_ASSERT("[TT-366-1480] StyleCascade::LookupAndApply %d %d %d %d",
priority ? (int)priority->GetPosition() : -1,
priority ? (int)priority->GetGeneration() : -1,
priority ? (int)priority->GetOrigin() : -1,
name.Id());
if (!priority)
return;

Expand All @@ -660,6 +667,11 @@ void StyleCascade::LookupAndApplyValue(const CSSProperty& property,
void StyleCascade::LookupAndApplyDeclaration(const CSSProperty& property,
CascadePriority* priority,
CascadeResolver& resolver) {
REPLAY_ASSERT("[TT-366-1480] StyleCascade::LookupAndApplyDeclaration %d %d %d",
property.PropertyID(),
(int)priority->GetGeneration(),
(int)resolver.generation_
);
if (priority->GetGeneration() >= resolver.generation_) {
// Already applied this generation.
return;
Expand Down Expand Up @@ -787,6 +799,12 @@ const CSSValue* StyleCascade::Resolve(const CSSProperty& property,
const CSSValue* StyleCascade::ResolveSubstitutions(const CSSProperty& property,
const CSSValue& value,
CascadeResolver& resolver) {
REPLAY_ASSERT("[TT-366-1480] StyleCascade::ResolveSubstitutions %d %d %d %d %d",
property.PropertyID(),
(int)value.ReplayGetClassType(),
!!DynamicTo<CSSCustomPropertyDeclaration>(value),
!!DynamicTo<CSSVariableReferenceValue>(value),
!!DynamicTo<cssvalue::CSSPendingSubstitutionValue>(value));
if (const auto* v = DynamicTo<CSSCustomPropertyDeclaration>(value))
return ResolveCustomProperty(property, *v, resolver);
if (const auto* v = DynamicTo<CSSVariableReferenceValue>(value))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1874,6 +1874,9 @@ StyleResolver::CacheSuccess StyleResolver::ApplyMatchedCache(
const MatchResult& match_result) {
const Element& element = state.GetElement();

REPLAY_ASSERT("[TT-366-1480] StyleResolver::ApplyMatchedCache %d",
element.RecordReplayId());

MatchedPropertiesCache::Key key(match_result);

bool is_inherited_cache_hit = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ const CSSValue& StyleResolverState::ResolveLightDarkPair(
}

void StyleResolverState::UpdateFont() {
recordreplay::Assert("[RUN-1436-2226] StyleResolverState::UpdateFont %d",
GetElement().RecordReplayId());
REPLAY_ASSERT("[TT-366-1480] StyleResolverState::UpdateFont %d",
GetElement().RecordReplayId());
GetFontBuilder().CreateFont(StyleRef(), ParentStyle());
SetConversionFontSizes(
CSSToLengthConversionData::FontSizes(Style(), RootElementStyle()));
Expand Down
7 changes: 3 additions & 4 deletions third_party/blink/renderer/core/dom/document.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8749,9 +8749,6 @@ void Document::CountUse(mojom::WebFeature feature) const {
}

void Document::CountUse(mojom::WebFeature feature) {
REPLAY_ASSERT("[TT-366-1467] Document::CountUse %d %d",
!!execution_context_,
feature);
if (execution_context_)
execution_context_->CountUse(feature);
}
Expand All @@ -8762,7 +8759,9 @@ void Document::CountDeprecation(mojom::WebFeature feature) {
}

void Document::CountProperty(CSSPropertyID property) const {
REPLAY_ASSERT("[TT-366-1467] Document::CountProperty %d %d",
REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1480] Document::CountProperty %d %d %d %d",
domWindow() ? domWindow()->RecordReplayId() : -1,
GetFrame() ? GetFrame()->RecordReplayId() : -1,
!!Loader(),
property);
if (DocumentLoader* loader = Loader()) {
Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2294,6 +2294,11 @@ void Element::setAttribute(const QualifiedName& name,
DISABLE_CFI_PERF
void Element::AttributeChanged(const AttributeModificationParams& params) {
const QualifiedName& name = params.name;
REPLAY_ASSERT("[TT-366-1480] Element::AttributeChanged %d %s=%s",
params.reason,
name.LocalName().Utf8().c_str(),
params.new_value.Utf8().c_str()
);
if (name == html_names::kSlotAttr && params.old_value != params.new_value) {
if (ShadowRoot* root = ShadowRootOfParent())
root->DidChangeHostChildSlotName(params.old_value, params.new_value);
Expand Down
3 changes: 0 additions & 3 deletions third_party/blink/renderer/core/frame/local_dom_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -725,9 +725,6 @@ void LocalDOMWindow::AddInspectorIssue(AuditsIssue issue) {
}

void LocalDOMWindow::CountUse(mojom::WebFeature feature) {
REPLAY_ASSERT("[TT-366-1467] LocalDOMWindow::CountUse %d %d",
!!GetFrame(),
GetFrame() ? !!GetFrame()->Loader().GetDocumentLoader() : -1);
if (!GetFrame())
return;
if (auto* loader = GetFrame()->Loader().GetDocumentLoader())
Expand Down
6 changes: 0 additions & 6 deletions third_party/blink/renderer/core/frame/use_counter_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,6 @@ void UseCounterImpl::Count(const UseCounterFeature& feature,
if (recordreplay::AreEventsDisallowed("UseCounterImpl::Count"))
return;

REPLAY_ASSERT("[TT-366-1467] UseCounterImpl::Count %d %u %d %d",
!!source_frame,
mute_count_,
feature.type(),
feature.value());

if (!source_frame)
return;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,10 @@ void HTMLTreeBuilder::ConstructTree(AtomicHTMLToken* token) {
else
ProcessToken(token);

REPLAY_ASSERT("[TT-366-1480] HTMLTreeBuilder::ConstructTree %d %d",
parser_->IsDetached(),
tree_.HasPendingTasks());

if (parser_->IsDetached())
return;

Expand Down
74 changes: 2 additions & 72 deletions third_party/blink/renderer/platform/context_lifecycle_notifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,96 +23,26 @@ bool ContextLifecycleNotifier::IsContextDestroyed() const {
void ContextLifecycleNotifier::AddContextLifecycleObserver(
ContextLifecycleObserver* observer) {
observers_.AddObserver(observer);

if (recordreplay::IsRecordingOrReplaying("values") && recordreplay::IsReplaying())
replay_observers_.push_back(observer);
}

void ContextLifecycleNotifier::RemoveContextLifecycleObserver(
ContextLifecycleObserver* observer) {
DCHECK(observers_.HasObserver(observer));
observers_.RemoveObserver(observer);

for (wtf_size_t i = 0; i < replay_observers_.size(); i++) {
if (replay_observers_[i] == observer) {
replay_observers_.EraseAt(i);
break;
}
}
}

void ContextLifecycleNotifier::NotifyContextDestroyed() {
context_destroyed_ = true;

ScriptForbiddenScope forbid_script;
HeapVector<Member<ContextLifecycleObserver>> observers;
observers_.ForEachObserver([&](ContextLifecycleObserver* observer) {
observers.push_back(observer);
observers_.ForEachObserver([](ContextLifecycleObserver* observer) {
Copy link
Author

@Domiii Domiii Jul 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: Undid outdated changes. This looks like a very old version of assuring determinism of a HeapObserverSet. I already fixed most of that inside HeapObserverSet itself a long time ago. I added the additional "leaking on Remove" part down below.

observer->NotifyContextDestroyed();
});
observers_.Clear();

std::sort(observers.begin(), observers.end(),
recordreplay::CompareMemberByPointerId<Member<ContextLifecycleObserver>>());

// When replaying, notify the same observers in the same order which were
// notified when recording. Because of the use of weak pointers in the
// HeapObserverSet the set contents can vary, so we manually record/replay
// the objects which should be notified. The replay_observers_ vector holds
// strong references on the observers when replaying so none of the observers
// we need to notify should already be collected.
if (recordreplay::IsRecordingOrReplaying("values", "ContextLifecycleNotifier::NotifyContextDestroyed") &&
recordreplay::FeatureEnabled("pointer-ids") &&
!recordreplay::AreEventsDisallowed()) {
size_t num_observers = recordreplay::RecordReplayValue("NotifyContextDestroyed NumObservers", observers.size());
int* observer_ids = new int[num_observers];

if (recordreplay::IsRecording()) {
for (wtf_size_t i = 0; i < observers.size(); i++) {
int id = recordreplay::PointerId(observers[i]);
CHECK(id);
observer_ids[i] = id;
}
}

recordreplay::RecordReplayBytes("ContextLifecycleNotifier::NotifyContextDestroyed ObserverIds",
observer_ids, num_observers * sizeof(int));

if (recordreplay::IsReplaying()) {
HeapVector<Member<ContextLifecycleObserver>> new_observers;
for (ContextLifecycleObserver* observer : observers) {
int id = recordreplay::PointerId(observer);
CHECK(id);
bool found = false;
for (wtf_size_t i = 0; i < num_observers; i++) {
if (observer_ids[i] == id) {
found = true;
break;
}
}
if (found)
new_observers.push_back(observer);
}

observers = std::move(new_observers);
}

delete[] observer_ids;
}

for (ContextLifecycleObserver* observer : observers) {
if (!recordreplay::AreEventsDisallowed()) {
recordreplay::Assert("ContextLifecycleNotifier::NotifyContextDestroyed #1 %d",
recordreplay::PointerId(observer));
}
observer->NotifyContextDestroyed();
}

replay_observers_.clear();
}

void ContextLifecycleNotifier::Trace(Visitor* visitor) const {
visitor->Trace(observers_);
visitor->Trace(replay_observers_);
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@ class PLATFORM_EXPORT ContextLifecycleNotifier : public GarbageCollectedMixin {
private:
HeapObserverSet<ContextLifecycleObserver> observers_;
bool context_destroyed_ = false;

// When replaying, strong references are held on all observers, which are
// cleared out after the context is destroyed. This ensures we can notify
// the same observers when replaying as when originally recording
HeapVector<Member<ContextLifecycleObserver>> replay_observers_;
};

} // namespace blink
Expand Down
Loading