Check if |SavedEvent| exists handling cancel values

In AudioParamTimeline::HandleCancelValues() function, the code expects the |saved_event| pointer from the next event to be non-null. There are corner cases where this is not true. Adding one more check on the saved event pointer fixes the crash. Locally confirmed the attached web test crashes without this fix. Bug: 913217 Test: webaudio/AudioParam/cancel-values-crash-913217.html Change-Id: I4760b939534ef92a2475087540e17c8abc784b3b Reviewed-on: https://chromium-review.googlesource.com/c/1374492 Reviewed-by: Kentaro Hara <[email protected]> Commit-Queue: Hongchan Choi <[email protected]> Cr-Commit-Position: refs/heads/master@{#616107}
feiyue1206 · Dec 12, 2018 · 46f2e65 · 46f2e65
1 parent 0ba5ea1
commit 46f2e65
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 2 deletions.
diff --git a/third_party/blink/renderer/modules/webaudio/audio_param_timeline.cc b/third_party/blink/renderer/modules/webaudio/audio_param_timeline.cc
@@ -1316,7 +1316,8 @@ AudioParamTimeline::HandleCancelValues(const ParamEvent* current_event,
   ParamEvent::Type next_event_type =
       next_event ? next_event->GetType() : ParamEvent::kLastType;
 
-  if (next_event && next_event->GetType() == ParamEvent::kCancelValues) {
+  if (next_event && next_event->GetType() == ParamEvent::kCancelValues &&
+      next_event->SavedEvent()) {
     float value1 = current_event->Value();
     double time1 = current_event->Time();
 

diff --git a/third_party/blink/renderer/modules/webaudio/audio_param_timeline.h b/third_party/blink/renderer/modules/webaudio/audio_param_timeline.h
@@ -367,7 +367,7 @@ class AudioParamTimeline {
 
   // Handle processing of CancelValue event. If cancellation happens, value2,
   // time2, and nextEventType will be updated with the new value due to
-  // cancellation.  The
+  // cancellation.  Note that |next_event| or its member can be null.
   std::tuple<float, double, ParamEvent::Type> HandleCancelValues(
       const ParamEvent* current_event,
       ParamEvent* next_event,

diff --git a/third_party/blink/web_tests/webaudio/AudioParam/cancel-values-crash-913217.html b/third_party/blink/web_tests/webaudio/AudioParam/cancel-values-crash-913217.html
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <title>
+      Test if canceling events after setTargetAtTime crashes
+    </title>
+    <script src="../../resources/testharness.js"></script>
+    <script src="../../resources/testharnessreport.js"></script>
+    <script src="../resources/audit-util.js"></script>
+    <script src="../resources/audit.js"></script>
+  </head>
+  <body>
+    <script id="layout-test-code">
+      // Arbitrary parameters for the offline context.
+      const sampleRate = 48000;
+      const renderDuration = 1;
+      const channels = 2;
+      const renderFrames = renderDuration * sampleRate;
+
+      const audit = Audit.createTaskRunner();
+
+      audit.define('crash', (task, should) => {
+        const context =
+            new OfflineAudioContext(channels, renderFrames, sampleRate);
+        const source = context.createConstantSource();
+        const analyser = context.createAnalyser();
+        source.connect(analyser);
+
+        // Scheduling the following at an earlier time than above causes the
+        // page to crash in 73.0.3638.0. See crbug.com/913217.
+        source.offset.setValueAtTime(1, context.currentTime);
+        source.offset.setTargetAtTime(0, context.currentTime + 2, 0.00001);
+        source.offset.cancelAndHoldAtTime(context.currentTime + 1.99999);
+
+        source.start(context.currentTime);
+
+        context.startRendering().then(() => {
+          // Should finish without a crash.
+          task.done();
+        });
+      });
+
+      audit.run();
+    </script>
+  </body>
+</html>