Assert access checks + more UseCounter code

DEPS

@@ -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': '31407bbf6a735ec3f362aeba6a347465a2f478ac',
   # 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.

REPLAY_BACKEND_REV

@@ -1 +1 @@
-6c7e9991dcf557721c0566c2c0d68ac94f2a5b10
+5612e56fd6a921da9f440c6a3158731d298613b9

third_party/blink/renderer/bindings/core/v8/binding_security.cc

@@ -131,8 +131,12 @@ bool CanAccessWindowInternal(
   // origin, depending on the model being used to allocate Frames between
   // processes. See https://crbug.com/601629.
   const auto* local_target_window = DynamicTo<LocalDOMWindow>(target_window);
-  if (!(accessing_window && local_target_window))
+  if (!(accessing_window && local_target_window)) {
+    REPLAY_ASSERT("[TT-366-1480] CanAccessWindowInternal A %d %d",
+      !!accessing_window,
+      !!local_target_window);
     return false;
+  }
 
   const SecurityOrigin* accessing_origin =
       accessing_window->GetSecurityOrigin();
@@ -256,6 +260,10 @@ bool BindingSecurity::ShouldAllowAccessTo(
     ErrorReportOption reporting_option) {
   DCHECK(target);
 
+  REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1480] BindingSecurity::ShouldAllowAccessTo A %d %d %d",
+    recordreplay::IsInReplayCode(),
+    !!accessing_window,
+    !!target->GetFrame());
   if (recordreplay::IsInReplayCode())
     return true;
 
@@ -270,6 +278,7 @@ bool BindingSecurity::ShouldAllowAccessTo(
   bool can_access = CanAccessWindow(accessing_window, target, reporting_option);
 
   if (!can_access && accessing_window) {
+    REPLAY_ASSERT("[TT-366-1480] BindingSecurity::ShouldAllowAccessTo B");
     UseCounter::Count(accessing_window->document(),
                       WebFeature::kCrossOriginPropertyAccess);
     if (target->opener() == accessing_window) {
@@ -287,6 +296,9 @@ bool BindingSecurity::ShouldAllowAccessTo(
     ExceptionState& exception_state) {
   DCHECK(target);
 
+  REPLAY_ASSERT("[TT-366-1480] BindingSecurity::ShouldAllowAccessTo %d",
+    !!target->DomWindow()->GetFrame());
+
   // TODO(https://crbug.com/723057): This is intended to match the legacy
   // behavior of when access checks revolved around Frame pointers rather than
   // DOMWindow pointers. This prevents web-visible behavior changes, since the
@@ -373,6 +385,9 @@ bool BindingSecurity::ShouldAllowAccessToFrame(
     const LocalDOMWindow* accessing_window,
     const Frame* target,
     ErrorReportOption reporting_option) {
+  REPLAY_ASSERT("[TT-366-1480] BindingSecurity::ShouldAllowAccessToFrame %d %d",
+    !target,
+    !target || !target->GetSecurityContext());
   if (!target || !target->GetSecurityContext())
     return false;
   return CanAccessWindow(accessing_window, target->DomWindow(),
@@ -389,6 +404,9 @@ bool ShouldAllowAccessToV8ContextInternal(
   // Workers and worklets do not support multiple contexts, so both of
   // |accessing_context| and |target_context| must be windows at this point.
 
+  REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1480] ShouldAllowAccessToV8ContextInternal A %d",
+    recordreplay::IsInReplayCode()
+  );
   if (recordreplay::IsInReplayCode())
     return true;
 
@@ -399,6 +417,7 @@ bool ShouldAllowAccessToV8ContextInternal(
     ReportOrThrowSecurityError(ToLocalDOMWindow(accessing_context), nullptr,
                                DOMWindow::CrossDocumentAccessPolicy::kAllowed,
                                error_report);
+    REPLAY_ASSERT("[TT-366-1480] ShouldAllowAccessToV8ContextInternal B");
     return false;
   }
   // Fast path for the most likely case.
@@ -409,6 +428,7 @@ bool ShouldAllowAccessToV8ContextInternal(
   // TODO(dcheng): Why doesn't this code just use DOMWindows throughout? Can't
   // we just always use ToLocalDOMWindow(context)?
   if (!target_frame) {
+    REPLAY_ASSERT("[TT-366-1480] ShouldAllowAccessToV8ContextInternal C");
     // Sandbox detached frames - they can't create cross origin objects.
     LocalDOMWindow* accessing_window = ToLocalDOMWindow(accessing_context);
     LocalDOMWindow* target_window = ToLocalDOMWindow(target_context);

third_party/blink/renderer/core/css/css_value.h

@@ -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,

third_party/blink/renderer/core/css/parser/css_parser_context.cc

@@ -229,7 +229,7 @@ KURL CSSParserContext::CompleteURL(const String& url) const {
 }
 
 void CSSParserContext::Count(WebFeature feature) const {
-  REPLAY_ASSERT("[TT-366-1467] CSSParserContext::Count feat %d %d",
+  REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1467] CSSParserContext::Count feat %d %d",
                 IsUseCounterRecordingEnabled(),
                 feature);
   if (IsUseCounterRecordingEnabled())
@@ -242,7 +242,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-1467] CSSParserContext::Count prop %d %d %d",
                 IsUseCounterRecordingEnabled(),
                 (int)mode,
                 property);

third_party/blink/renderer/core/css/parser/css_parser_impl.cc

@@ -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);
   }

third_party/blink/renderer/core/css/parser/css_selector_parser.cc

@@ -457,6 +457,8 @@ ArenaUniquePtr<CSSParserSelector> CSSSelectorParser::ConsumeRelativeSelector(
 
 ArenaUniquePtr<CSSParserSelector> CSSSelectorParser::ConsumeComplexSelector(
     CSSParserTokenRange& range) {
+  REPLAY_ASSERT("[TT-366-1480] CSSSelectorParser::ConsumeComplexSelector %u",
+    range.size());
   ArenaUniquePtr<CSSParserSelector> selector = ConsumeCompoundSelector(range);
   if (!selector)
     return nullptr;

third_party/blink/renderer/core/css/resolver/match_result.cc

@@ -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_
   );
 

third_party/blink/renderer/core/css/resolver/style_builder_converter.cc

@@ -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 %d",
+    (int)value.ReplayGetClassType());
   FontDescription::FamilyDescription desc(FontDescription::kNoFamily);
 
   if (const auto* system_font =

third_party/blink/renderer/core/css/resolver/style_cascade.cc

@@ -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_);
@@ -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;
 
@@ -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;
@@ -787,6 +799,9 @@ 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",
+    property.PropertyID(),
+    (int)value.ReplayGetClassType());
   if (const auto* v = DynamicTo<CSSCustomPropertyDeclaration>(value))
     return ResolveCustomProperty(property, *v, resolver);
   if (const auto* v = DynamicTo<CSSVariableReferenceValue>(value))

third_party/blink/renderer/core/css/resolver/style_resolver.cc

@@ -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;

third_party/blink/renderer/core/css/resolver/style_resolver_state.cc

@@ -244,7 +244,7 @@ const CSSValue& StyleResolverState::ResolveLightDarkPair(
 }
 
 void StyleResolverState::UpdateFont() {
-  recordreplay::Assert("[RUN-1436-2226] StyleResolverState::UpdateFont %d",
+  recordreplay::Assert("[TT-366-1480] StyleResolverState::UpdateFont %d",
                        GetElement().RecordReplayId());
   GetFontBuilder().CreateFont(StyleRef(), ParentStyle());
   SetConversionFontSizes(

third_party/blink/renderer/core/dom/document.cc

@@ -8749,7 +8749,7 @@ void Document::CountUse(mojom::WebFeature feature) const {
 }
 
 void Document::CountUse(mojom::WebFeature feature) {
-  REPLAY_ASSERT("[TT-366-1467] Document::CountUse %d %d",
+  REPLAY_ASSERT_MAYBE_EVENTS_DISALLOWED("[TT-366-1467] Document::CountUse %d %d",
                 !!execution_context_,
                 feature);
   if (execution_context_)
@@ -8762,7 +8762,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()) {

third_party/blink/renderer/core/frame/local_dom_window.cc

@@ -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())

third_party/blink/renderer/platform/context_lifecycle_notifier.cc

@@ -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) {
+    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

third_party/blink/renderer/platform/context_lifecycle_notifier.h

@@ -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