From 580a82eb29f13743b0799ad5efd5d78402519ad7 Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Sat, 12 Aug 2023 13:41:02 -0700 Subject: [PATCH] Use V8 IsInReplayCode API --- DEPS | 2 +- base/record_replay.cc | 17 ++++++++++++----- base/record_replay.h | 7 +++++++ .../bindings/core/v8/record_replay_interface.cc | 16 +++++----------- .../blink/renderer/core/frame/dom_window.cc | 2 ++ .../renderer/modules/storage/storage_area.cc | 6 +----- 6 files changed, 28 insertions(+), 22 deletions(-) diff --git a/DEPS b/DEPS index 7d85ecf622f4a7..ad2c417352bc5d 100644 --- a/DEPS +++ b/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': 'c86e580010c0337d25390d70e3ed9d251364b5a9', + 'v8_revision': '3ffb178192079d96695d5e2010c432dfd062dff4', # 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. diff --git a/base/record_replay.cc b/base/record_replay.cc index 6a0123c0ccec53..af9ac7749b8c52 100644 --- a/base/record_replay.cc +++ b/base/record_replay.cc @@ -40,6 +40,7 @@ namespace recordreplay { Macro(V8RecordReplayHasDisabledFeatures, (), (), bool, false) \ Macro(V8RecordReplayAreAssertsDisabled, (), (), bool, false) \ Macro(V8IsMainThread, (), (), bool, false) \ + Macro(V8RecordReplayIsInReplayCode, (), (), bool, false) \ Macro(V8RecordReplayHadMismatch, (), (), bool, false) #define ForEachV8APIVoid(Macro) \ @@ -111,7 +112,9 @@ namespace recordreplay { Macro(V8RecordReplayMaybeTerminate, \ (void (*callback)(void*), void* data), (callback, data)) \ Macro(V8RecordReplayGetCurrentJSStack, \ - (std::string* stackTrace), (stackTrace)) + (std::string* stackTrace), (stackTrace)) \ + Macro(V8RecordReplayEnterReplayCode, (), ()) \ + Macro(V8RecordReplayExitReplayCode, (), ()) #if BUILDFLAG(IS_WIN) @@ -493,12 +496,16 @@ int NewIdAnyThread(const char* name) { } bool IsInReplayCode() { - // Events are disallowed when running Replay's own scripts. - // FIXME Add to Recording API. - // https://linear.app/replay/issue/RUN-1502 - return IsReplaying() && AreEventsDisallowed(); + return V8RecordReplayIsInReplayCode(); } +AutoMarkReplayCode::AutoMarkReplayCode() { + V8RecordReplayEnterReplayCode(); +} + +AutoMarkReplayCode::~AutoMarkReplayCode() { + V8RecordReplayExitReplayCode(); +} void RecordReplayString(const char* why, std::string& str) { size_t length = RecordReplayValue(why, str.length()); diff --git a/base/record_replay.h b/base/record_replay.h index 5ba761a8fb8128..bf2452e802c4c2 100644 --- a/base/record_replay.h +++ b/base/record_replay.h @@ -110,8 +110,15 @@ void OnNavigationEvent(const char* kind, const char* url); int NewIdMainThread(const char* name); int NewIdAnyThread(const char* name); +// Return whether record/replay specific scripts are executing. bool IsInReplayCode(); +// Mark a region where record/replay specific scripts are executing. +struct AutoMarkReplayCode { + AutoMarkReplayCode(); + ~AutoMarkReplayCode(); +}; + // stl comparator that uses pointer IDs to compare elements when recording/replaying, // giving a deterministic sort order. struct CompareByPointerId { diff --git a/third_party/blink/renderer/bindings/core/v8/record_replay_interface.cc b/third_party/blink/renderer/bindings/core/v8/record_replay_interface.cc index 62c033622db612..220a24b3c439d5 100644 --- a/third_party/blink/renderer/bindings/core/v8/record_replay_interface.cc +++ b/third_party/blink/renderer/bindings/core/v8/record_replay_interface.cc @@ -129,16 +129,6 @@ const gSourceMapData = new Map(); try { - - - - - - - - - - // Save these before page code potentially overwrites them. const JSON_stringify = JSON.stringify; const JSON_parse = JSON.parse; @@ -4901,6 +4891,7 @@ void SetupRecordReplayCommands(v8::Isolate* isolate, LocalFrame* localFrame) { if (recordreplay::FeatureEnabled("collect-source-maps") && !TestEnv("RECORD_REPLAY_DISABLE_SOURCEMAP_COLLECTION")) { + recordreplay::AutoMarkReplayCode amrc; RunScript(isolate, context, gSourceMapScript, InternalScriptURL); } @@ -4911,14 +4902,16 @@ void SetupRecordReplayCommands(v8::Isolate* isolate, LocalFrame* localFrame) { } if (recordreplay::IsReplaying()) { + recordreplay::AutoMarkReplayCode amrc; recordreplay::AutoDisallowEvents disallow("SetupRecordReplayCommands"); RunScript(isolate, context, gReplayScript, InternalScriptURL); } } void OnNewRootFrame(v8::Isolate* isolate, LocalFrame* localFrame) { + recordreplay::AutoMarkReplayCode amrc; v8::Local context = isolate->GetCurrentContext(); - + // 1. Register navigation event. if (localFrame->GetDocument()->Url().ProtocolIsInHTTPFamily()) { recordreplay::OnNavigationEvent( @@ -4941,6 +4934,7 @@ void OnNewRootFrame(v8::Isolate* isolate, LocalFrame* localFrame) { } void OnNewWindow2(v8::Isolate* isolate, LocalFrame* localFrame) { + recordreplay::AutoMarkReplayCode amrc; v8::Local context = isolate->GetCurrentContext(); RunScript(isolate, context, gOnNewWindowScript, "record-replay-devtools-OnNewWindow"); diff --git a/third_party/blink/renderer/core/frame/dom_window.cc b/third_party/blink/renderer/core/frame/dom_window.cc index 38ecf72a19dd38..8852aa366e4cc1 100644 --- a/third_party/blink/renderer/core/frame/dom_window.cc +++ b/third_party/blink/renderer/core/frame/dom_window.cc @@ -263,6 +263,8 @@ String DOMWindow::SanitizedCrossDomainAccessErrorMessage( if (accessing_window_url.IsNull()) return String(); + recordreplay::Trace("[RUN-1990] DOMWindow::SanitizedCrossDomainAccessErrorMessage"); + const SecurityOrigin* active_origin = accessing_window->GetSecurityOrigin(); String message; if (cross_document_access == CrossDocumentAccessPolicy::kDisallowed) { diff --git a/third_party/blink/renderer/modules/storage/storage_area.cc b/third_party/blink/renderer/modules/storage/storage_area.cc index 9aed892f89c4e9..c7ec70d8201e59 100644 --- a/third_party/blink/renderer/modules/storage/storage_area.cc +++ b/third_party/blink/renderer/modules/storage/storage_area.cc @@ -170,11 +170,7 @@ bool StorageArea::Contains(const String& key, void StorageArea::NamedPropertyEnumerator(Vector& names, ExceptionState& exception_state) { if (!cached_area_->memory_used() && // This means either empty or not loaded - v8::recordreplay::IsReplaying() && - v8::recordreplay::AreEventsDisallowed()) { - // TODO: Use `IsReplayCode` instead - - // https://linear.app/replay/issue/RUN-1502 - + recordreplay::IsInReplayCode()) { // This ignores crash-inducing side effects observed during interceptor key collection // when handling commands. // See: https://linear.app/replay/issue/RUN-1315#comment-26f96699