[Media Controls] Prevent crash when video is removed on click

This CL adds an isConnected check in the overlay play button's click positioning check to prevent a crash when trying to access the layout data after it's been removed. Bug: 870490, 881772 Change-Id: Ic005ae9d3acc2945ed67645bfe98a237c89e38f0 Reviewed-on: https://chromium-review.googlesource.com/1217065 Reviewed-by: Becca Hughes <[email protected]> Commit-Queue: Tommy Steimel <[email protected]> Cr-Commit-Position: refs/heads/master@{#590058}
feiyue1206 · Sep 10, 2018 · 98a5ecf · 98a5ecf
1 parent a1f0a5b
commit 98a5ecf
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 2 deletions.
diff --git a/third_party/WebKit/LayoutTests/media/controls/remove-on-click-does-not-crash.html b/third_party/WebKit/LayoutTests/media/controls/remove-on-click-does-not-crash.html
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<title>Test that when a video is removed on click it does not crash.</title>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../media-controls.js"></script>
+<video controls width=500 preload=none src="../content/60_sec_video.webm"></video>
+<script>
+async_test(t => {
+  const video = document.querySelector('video');
+
+  video.addEventListener('click', t.step_func_done(() => {
+    video.parentNode.removeChild(video);
+  }), { once: true });
+
+  video.addEventListener('loadedmetadata', t.step_func(() => {
+    const coords = getOverlayPlayButtonEdgeCoordinates();
+    singleTapAtCoordinates(coords[0], coords[1]);
+  }));
+
+  function getOverlayPlayButtonEdgeCoordinates() {
+    const overlayPlayBtn = mediaControlsOverlayPlayButton(video);
+    const rect = overlayPlayBtn.getBoundingClientRect();
+    const x = rect.left + 5;
+    const y = rect.top + 5;
+    return [x, y];
+  }
+
+  video.load();
+});
+</script>
+</html>
diff --git a/...ink/renderer/modules/media_controls/elements/media_control_overlay_play_button_element.cc b/...ink/renderer/modules/media_controls/elements/media_control_overlay_play_button_element.cc
@@ -296,9 +296,12 @@ bool MediaControlOverlayPlayButtonElement::ShouldCausePlayPause(
 
 bool MediaControlOverlayPlayButtonElement::IsMouseEventOnInternalButton(
     const MouseEvent& mouse_event) const {
-  // If no position data available, default to yes.
-  if (!mouse_event.HasPosition())
+  // If we don't have the necessary pieces to calculate whether the event is
+  // within the bounds of the button, default to yes.
+  if (!mouse_event.HasPosition() || !isConnected() ||
+      !GetDocument().GetLayoutView()) {
     return true;
+  }
 
   // Find the zoom-adjusted internal button bounding box.
   DOMRect* box = internal_button_->getBoundingClientRect();