[TT-1112] Fix the sourcemap handler script

.buildkite/pipeline.yml

@@ -216,6 +216,7 @@ steps:
     depends_on:
       - "build-chromium-linux-x86_64"
     build:
+      branch: dominik/tt-1261-recorder-crashes-from-save-examplests-are-not-uploaded
       env:
         OS: "linux"
         ARCH: "x86_64"
@@ -231,6 +232,7 @@ steps:
     depends_on:
       - "build-chromium-mac-arm64"
     build:
+      branch: dominik/tt-1261-recorder-crashes-from-save-examplests-are-not-uploaded
       env:
         OS: "macos"
         ARCH: "arm64"

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': '7e105ba00a264f31ed04d5bada4d3aa63be1e0f0',
+  'v8_revision': 'ea1ffeb18c92e9a1c7f3fda7307bc7c9825d1fb5',
   # 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 @@
-9e69b244fd0973ea3f8fb46fc9eb2ce39fa615a8
+63be1e1d8a17bdfe09f570f2eed77136ec29ae81

base/logging.cc

@@ -127,29 +127,46 @@ typedef FILE* FileHandle;
 #include <windows.h>
 #endif
 
+static void* REPLAY_NO_HANDLE = reinterpret_cast<void*>(1);
+
 static void* LookupRecordReplaySymbol(const char* name) {
 #if !BUILDFLAG(IS_WIN)
   void* fnptr = dlsym(RTLD_DEFAULT, name);
 #else
   HMODULE module = GetModuleHandleA("windows-recordreplay.dll");
   void* fnptr = module ? (void*)GetProcAddress(module, name) : nullptr;
 #endif
-  return fnptr ? fnptr : reinterpret_cast<void*>(1);
+  return fnptr ? fnptr : REPLAY_NO_HANDLE;
 }
 
-static void RecordReplayPrint(const char* aFormat, ...) {
-  static void* fnptr;
-  if (!fnptr) {
-    fnptr = LookupRecordReplaySymbol("RecordReplayPrint");
+static void* g_record_replay_print_ptr;
+static bool HasReplayModule() {
+  if (!g_record_replay_print_ptr) {
+    g_record_replay_print_ptr = LookupRecordReplaySymbol("RecordReplayPrint");
   }
-  if (fnptr != reinterpret_cast<void*>(1)) {
+  return g_record_replay_print_ptr != REPLAY_NO_HANDLE;
+}
+
+static void RecordReplayPrint(const char* aFormat, ...) {
+  if (HasReplayModule()) {
     va_list ap;
     va_start(ap, aFormat);
-    reinterpret_cast<void(*)(const char*, va_list)>(fnptr)(aFormat, ap);
+    reinterpret_cast<void(*)(const char*, va_list)>(g_record_replay_print_ptr)(aFormat, ap);
     va_end(ap);
   }
 }
 
+static bool RecordReplayIsReplaying() {
+  static void* fnptr;
+  if (!fnptr) {
+    fnptr = LookupRecordReplaySymbol("RecordReplayIsReplaying");
+  }
+  if (fnptr != REPLAY_NO_HANDLE) {
+    return reinterpret_cast<bool(*)()>(fnptr)();
+  }
+  return false;
+}
+
 namespace logging {
 
 namespace {
@@ -743,7 +760,10 @@ LogMessage::~LogMessage() {
   size_t stack_start = stream_.str().length();
 #if !defined(OFFICIAL_BUILD) && !BUILDFLAG(IS_NACL) && !defined(__UCLIBC__) && \
     !BUILDFLAG(IS_AIX)
-  if (severity_ == LOGGING_FATAL && !base::debug::BeingDebugged()) {
+
+  // Prevent calling |BeingDebugged| since it tries to create a debugger process.
+  // That attempt will cause a crash when replaying when events are disallowed.
+  if (severity_ == LOGGING_FATAL && !RecordReplayIsReplaying() && !base::debug::BeingDebugged()) {
     // Include a stack trace on a fatal, unless a debugger is attached.
     base::debug::StackTrace stack_trace;
     stream_ << std::endl;  // Newline to separate from log message.
@@ -766,8 +786,15 @@ LogMessage::~LogMessage() {
 #endif
   stream_ << std::endl;
   std::string str_newline(stream_.str());
-
-  RecordReplayPrint("LogMessage %s", str_newline.c_str());
+
+  if (severity_ == LOGGING_FATAL) {
+    // TODO: We actually want to crash with the right reason, but we don't have
+    // access to V8RecordReplayCrash.
+    // RecordReplayCrash("FATAL: %s", str_newline.c_str());
+    RecordReplayPrint("FATAL: %s", str_newline.c_str());
+  } else {
+    RecordReplayPrint("LogMessage [%d/%d] %s", severity_, LOGGING_NUM_SEVERITIES-1, str_newline.c_str());
+  }
 
   TRACE_LOG_MESSAGE(
       file_, base::StringPiece(str_newline).substr(message_start_), line_);
@@ -981,7 +1008,7 @@ LogMessage::~LogMessage() {
       // information, and displaying message boxes when the application is
       // hosed can cause additional problems.
 #ifndef NDEBUG
-      if (!base::debug::BeingDebugged()) {
+      if (!RecordReplayIsReplaying() && !base::debug::BeingDebugged()) {
         // Displaying a dialog is unnecessary when debugging and can complicate
         // debugging.
         DisplayDebugMessageInDialog(stream_.str());

build.js

@@ -167,11 +167,12 @@ function spawnChecked(cmd, args, options) {
 
   const rv = spawnSync(cmd, args, options);
 
-  if (rv.status != 0 || rv.error) {
+  if (rv.status || rv.error || rv.signal) {
+    const errMsg = ` Spawned process "${prettyCmd}" failed: status=${rv.status || ""}, signal=${rv.signal || ""}, err=${rv.error || ""}`;
     console.error(
-      `Process failed: err=${rv.error || ""}, signal=${rv.signal || ""}`
+      errMsg
     );
-    throw new Error(`Spawned process failed with exit code ${rv.status}`);
+    throw new Error(errMsg);
   }
 
   return rv;

replay-assets/replay_command_handlers.js

@@ -14,9 +14,6 @@ const {
   hasDiverged,
   setCDPMessageCallback,
   sendCDPMessage: sendCDPMessageRaw,
-  setCommandCallback,
-  setClearPauseDataCallback,
-  addNewScriptHandler,
   getCurrentError,
 
   layoutDom,
@@ -53,7 +50,7 @@ const {
 // utils.js
 ///////////////////////////////////////////////////////////////////////////////
 
-// Some of these are duplicated in gSourceMapScript, so watch out when making
+// Some of these are duplicated in replay_sourcemap_handler, so watch out when making
 // modifications to update both versions...
 
 function isFunction(val) {
@@ -144,13 +141,11 @@ function isValidBaseURL(url) {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
-// message.js
+// CDP Handlers.
 ///////////////////////////////////////////////////////////////////////////////
 
-function initMessages() {
+function initCdp() {
   setCDPMessageCallback(messageCallback);
-  setCommandCallback(commandCallback);
-  setClearPauseDataCallback(clearPauseDataCallback);
 }
 
 let gNextMessageId = 1;
@@ -232,7 +227,7 @@ function sendCDPMessage(method, params) {
 const sendMessage = sendCDPMessage;
 
 
-function addEventListener(method, callback) {
+function addCDPEventListener(method, callback) {
   gEventListeners.set(method, callback);
 }
 
@@ -323,19 +318,23 @@ function executeCommand(method, params) {
   VerboseCommands && log(`[Command ${method}] Handling command, params=${JSON_stringify(params)}...`);
   const result = CommandCallbacks[method](params);
   VerboseCommands && log(`[Command ${method}] Handled command, result=${JSON_stringify(result)}`);
+  if (!result) {
+    // NOTE: CommandCallback expects a result.
+    throw new Error(`[Command ${method}] Did not return a result.`);
+  }
   return result;
 }
 
-function commandCallback(method, params) {
-  if (!CommandCallbacks[method]) {
-    log(`[RuntimeError][Command ${method}] Missing command callback: ${method}`);
+function commandCallback(commandName, params) {
+  if (!CommandCallbacks[commandName]) {
+    log(`[RuntimeError][Command ${commandName}] Missing command callback: ${commandName}`);
     return {};
   }
 
   try {
-    return executeCommand(method, params);
+    return executeCommand(commandName, params);
   } catch (e) {
-    log(`[RuntimeError][Command ${method}]${getAliveLabel()} ${e?.stack || e}`);
+    log(`[RuntimeError][Command ${commandName}]${getAliveLabel()} ${e?.stack || e}`);
     // Pass the error up to V8; it can (for now) decide how to handle itself, whether
     // it should crash or not, etc.  Eventually, the caller of the command should make
     // that decision.
@@ -426,21 +425,6 @@ function Target_getCurrentMessageContents() {
   };
 }
 
-addNewScriptHandler((scriptId, sourceURL, relativeSourceMapURL) => {
-  if (!relativeSourceMapURL)
-    return;
-
-  const urls = getSourceMapURLs(sourceURL, relativeSourceMapURL);
-  if (!urls)
-    return;
-
-  const { sourceMapURL, sourceMapBaseURL } = urls;
-  gSourceMapData.set(scriptId, {
-    url: sourceMapURL,
-    baseUrl: sourceMapBaseURL
-  });
-}, /* disallowEvents */ true);
-
 function Target_getSourceMapURL({ sourceId }) {
   return gSourceMapData.get(sourceId) || {};
 }
@@ -3317,28 +3301,52 @@ function wrapReplayApiFunction(fn) {
   };
 }
 
+/** ###########################################################################
+ * ReplayJs: Internal event handling.
+ * ##########################################################################*/
+
+function handleNewScript(scriptId, sourceURL, relativeSourceMapURL) {
+  if (!relativeSourceMapURL)
+    return;
+
+  const urls = getSourceMapURLs(sourceURL, relativeSourceMapURL);
+  if (!urls)
+    return;
+
+  const { sourceMapURL, sourceMapBaseURL } = urls;
+  gSourceMapData.set(scriptId, {
+    url: sourceMapURL,
+    baseUrl: sourceMapBaseURL
+  });
+}
+
+function initializeReplayJsEvents(ReplayJsEventEmitter) {
+  ReplayJsEventEmitter.on("command", commandCallback);
+  ReplayJsEventEmitter.on("clearPauseDataCallback", clearPauseDataCallback);
+  ReplayJsEventEmitter.on("newScriptEventsDisallowed", handleNewScript);
+}
 
 ///////////////////////////////////////////////////////////////////////////////
 // main.js
 ///////////////////////////////////////////////////////////////////////////////
 
 patchReplayApi();
-initMessages();
-addEventListener("Runtime.consoleAPICalled", onConsoleAPICall);
-addEventListener("Runtime.executionContextCreated", ({ context }) => {
+initCdp();
+addCDPEventListener("Runtime.consoleAPICalled", onConsoleAPICall);
+addCDPEventListener("Runtime.executionContextCreated", ({ context }) => {
   gExecutionContexts.set(context.id, context);
   for (const callback of gContextChangeCallbacks) {
     callback(context, "add");
   }
 });
-addEventListener("Runtime.executionContextDestroyed", ({ executionContextId }) => {
+addCDPEventListener("Runtime.executionContextDestroyed", ({ executionContextId }) => {
   const context = gExecutionContexts.get(executionContextId);
   for (const callback of gContextChangeCallbacks) {
     callback(context, "remove");
   }
   gExecutionContexts.delete(executionContextId);
 });
-addEventListener("Runtime.executionContextsCleared", () => {
+addCDPEventListener("Runtime.executionContextsCleared", () => {
   for (const context of gExecutionContexts.values()) {
     for (const callback of gContextChangeCallbacks) {
       callback(context, "remove");
@@ -3348,4 +3356,6 @@ addEventListener("Runtime.executionContextsCleared", () => {
 });
 sendCDPMessage("Runtime.enable");
 
-})();
+// Script return value
+return initializeReplayJsEvents;
+})()

replay-assets/replay_init.js

@@ -0,0 +1,37 @@
+(() => {
+/** ###########################################################################
+ * ReplayJs: Internal event handling.
+ * ##########################################################################*/
+
+const ReplayJsEventEmitterPrototype = {
+  on(event, cb) {
+    this._callbacks[event] ||= [];
+    this._callbacks[event].push(cb);
+  },
+
+  emit(event, ...args) {
+    let cbs = this._callbacks[event];
+    if (cbs) {
+      cbs.forEach((cb) => cb(...args));
+    }
+  },
+
+  emitWithResult(event, ...args) {
+    let cbs = this._callbacks[event];
+    if (!cbs?.length) {
+      // If the caller expects a return value, there must be at least one callback registered.
+      throw new Error(`ReplayJsEvent_emitWithResult_failed_unknown_event: ${event}`);
+    }
+    const rv = cbs[0](...args);
+    return rv;
+  },
+};
+
+function initializeReplayJsEvents(ReplayJsEventEmitter) {
+  // Set up the event emitter.
+  Object.assign(ReplayJsEventEmitter, ReplayJsEventEmitterPrototype);
+  ReplayJsEventEmitter._callbacks = {};
+}
+
+return initializeReplayJsEvents;
+})();