Commit 936898b9 authored by Tommy Steimel's avatar Tommy Steimel Committed by Commit Bot

[Media Controls] Reland Prevent ComputedStyleRef crash

This CL re-adds the null checks for internal_button_ and
GetComputedStyle on the overlay play button. These were added to
prevent a crash, then removed when the crash was not fixed and replaced
with a different null check. However, it looks like we actually need
these too, so we're adding them back.

Bug: 870490
Change-Id: I1c6ad4b3f1dc36c7a2149a06c4f42f8ca00fb589
Reviewed-on: https://chromium-review.googlesource.com/1187710Reviewed-by: default avatarBecca Hughes <beccahughes@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585719}
parent 4663663d
......@@ -296,13 +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())
return true;
// If there is no layout view, default to yes.
if (!GetDocument().GetLayoutView())
// 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() || !internal_button_ ||
!GetDocument().GetLayoutView() || !GetComputedStyle()) {
return true;
}
// Find the zoom-adjusted internal button bounding box.
DOMRect* box = internal_button_->getBoundingClientRect();
......
......@@ -7,6 +7,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/core/css/css_property_value_set.h"
#include "third_party/blink/renderer/core/dom/events/event.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
#include "third_party/blink/renderer/core/event_type_names.h"
#include "third_party/blink/renderer/core/events/mouse_event.h"
#include "third_party/blink/renderer/core/html/html_html_element.h"
......@@ -17,11 +18,28 @@
namespace blink {
class MediaControlOverlayPlayButtonElementTest : public PageTestBase {
class MediaControlOverlayPlayButtonElementTest
: public PageTestBase,
private ScopedModernMediaControlsForTest {
public:
MediaControlOverlayPlayButtonElementTest()
: ScopedModernMediaControlsForTest(true) {}
void SetUp() final {
// Create page and instance of AnimatedArrow to run tests on.
// Create page with video element with controls.
PageTestBase::SetUp();
HTMLVideoElement* media_element = HTMLVideoElement::Create(GetDocument());
media_element->SetBooleanAttribute(HTMLNames::controlsAttr, true);
GetDocument().body()->AppendChild(media_element);
// Create instance of MediaControlOverlayPlayButtonElement for tests.
MediaControlsImpl* media_controls =
static_cast<MediaControlsImpl*>(media_element->GetMediaControls());
ASSERT_NE(nullptr, media_controls);
overlay_play_button_ =
new MediaControlOverlayPlayButtonElement(*media_controls);
// Create instance of AnimatedArrow to run tests on.
arrow_element_ = new MediaControlOverlayPlayButtonElement::AnimatedArrow(
"test", GetDocument());
GetDocument().body()->AppendChild(arrow_element_);
......@@ -47,6 +65,16 @@ class MediaControlOverlayPlayButtonElementTest : public PageTestBase {
GetElementById("arrow-3")->DispatchEvent(*event);
}
void RemoveInternalButton() {
overlay_play_button_->internal_button_ = nullptr;
}
void EnsureNoComputedStyle() {
// Due to the nature of the test setup, we already get a null computedstyle
// for free :). Just make sure it's actually not there.
ASSERT_EQ(nullptr, overlay_play_button_->GetComputedStyle());
}
Document& CreateTestDocumentWithBody() {
Document* document = Document::CreateForTest();
HTMLHtmlElement* html = HTMLHtmlElement::Create(*document);
......@@ -118,9 +146,20 @@ TEST_F(MediaControlOverlayPlayButtonElementTest, ShowIncrementsCounter) {
ExpectPresentAndShown();
}
TEST_F(MediaControlOverlayPlayButtonElementTest,
KeepEventInNodeWithoutInternalButtonDoesntCrash) {
RemoveInternalButton();
SimulateKeepEventInNode();
}
TEST_F(MediaControlOverlayPlayButtonElementTest,
KeepEventInNodeWithoutComputedStyleDoesntCrash) {
EnsureNoComputedStyle();
SimulateKeepEventInNode();
}
TEST_F(MediaControlOverlayPlayButtonElementTest,
KeepEventInNodeWithoutLayoutViewDoesntCrash) {
ScopedModernMediaControlsForTest enable_modern_media_controls(true);
Document& document_without_layout_view = CreateTestDocumentWithBody();
CreateTestOverlayPlayButton(document_without_layout_view);
SimulateKeepEventInNode();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment