Commit 0c629790 authored by Tommy Steimel's avatar Tommy Steimel Committed by Commit Bot

[Media Controls] Prevent ComputedStyleRef crash in overlay play button

This CL adds a check in MediaControlOverlayPlayButtonElement that
prevents a call to ComputedStyleRef is ComputedStyle is null. This
prevents a crash we're seeing in certain cases on Windows and Mac.

Bug: 870490
Change-Id: Ia45a7239084b267ce8a352fc0a825b5c992c5eea
Reviewed-on: https://chromium-review.googlesource.com/1173471Reviewed-by: default avatarHayato Ito <hayato@chromium.org>
Reviewed-by: default avatarBecca Hughes <beccahughes@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583523}
parent a7885a8a
...@@ -3893,6 +3893,10 @@ void Element::StoreNonLayoutObjectComputedStyle( ...@@ -3893,6 +3893,10 @@ void Element::StoreNonLayoutObjectComputedStyle(
EnsureElementRareData().SetComputedStyle(std::move(style)); EnsureElementRareData().SetComputedStyle(std::move(style));
} }
void Element::ClearComputedStyle() {
EnsureElementRareData().ClearComputedStyle();
}
AtomicString Element::ComputeInheritedLanguage() const { AtomicString Element::ComputeInheritedLanguage() const {
const Node* n = this; const Node* n = this;
AtomicString value; AtomicString value;
......
...@@ -547,6 +547,8 @@ class CORE_EXPORT Element : public ContainerNode { ...@@ -547,6 +547,8 @@ class CORE_EXPORT Element : public ContainerNode {
bool ShouldStoreNonLayoutObjectComputedStyle(const ComputedStyle&) const; bool ShouldStoreNonLayoutObjectComputedStyle(const ComputedStyle&) const;
void StoreNonLayoutObjectComputedStyle(scoped_refptr<ComputedStyle>); void StoreNonLayoutObjectComputedStyle(scoped_refptr<ComputedStyle>);
void ClearComputedStyle();
// Methods for indicating the style is affected by dynamic updates (e.g., // Methods for indicating the style is affected by dynamic updates (e.g.,
// children changing, our position changing in our sibling list, etc.) // children changing, our position changing in our sibling list, etc.)
bool StyleAffectedByEmpty() const { bool StyleAffectedByEmpty() const {
......
...@@ -300,6 +300,11 @@ bool MediaControlOverlayPlayButtonElement::IsMouseEventOnInternalButton( ...@@ -300,6 +300,11 @@ bool MediaControlOverlayPlayButtonElement::IsMouseEventOnInternalButton(
if (!mouse_event.HasPosition()) if (!mouse_event.HasPosition())
return true; return true;
// TODO(https://crbug.com/873839): In what cases do we hit this?
// If the internal button or computed style are unavailable, default to yes.
if (!internal_button_ || !GetComputedStyle())
return true;
// Find the zoom-adjusted internal button bounding box. // Find the zoom-adjusted internal button bounding box.
DOMRect* box = internal_button_->getBoundingClientRect(); DOMRect* box = internal_button_->getBoundingClientRect();
float zoom = ComputedStyleRef().EffectiveZoom() / float zoom = ComputedStyleRef().EffectiveZoom() /
......
...@@ -8,15 +8,30 @@ ...@@ -8,15 +8,30 @@
#include "third_party/blink/renderer/core/css/css_property_value_set.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/events/event.h"
#include "third_party/blink/renderer/core/event_type_names.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/media/html_video_element.h"
#include "third_party/blink/renderer/core/testing/page_test_base.h" #include "third_party/blink/renderer/core/testing/page_test_base.h"
#include "third_party/blink/renderer/modules/media_controls/media_controls_impl.h"
namespace blink { namespace blink {
class MediaControlOverlayPlayButtonElementTest : public PageTestBase { class MediaControlOverlayPlayButtonElementTest : public PageTestBase {
public: public:
void SetUp() final { void SetUp() final {
// Create page and instance of AnimatedArrow to run tests on. // Create page with video element with controls.
PageTestBase::SetUp(); 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( arrow_element_ = new MediaControlOverlayPlayButtonElement::AnimatedArrow(
"test", GetDocument()); "test", GetDocument());
GetDocument().body()->AppendChild(arrow_element_); GetDocument().body()->AppendChild(arrow_element_);
...@@ -42,6 +57,22 @@ class MediaControlOverlayPlayButtonElementTest : public PageTestBase { ...@@ -42,6 +57,22 @@ class MediaControlOverlayPlayButtonElementTest : public PageTestBase {
GetElementById("arrow-3")->DispatchEvent(*event); GetElementById("arrow-3")->DispatchEvent(*event);
} }
void RemoveInternalButton() {
overlay_play_button_->internal_button_ = nullptr;
}
void RemoveComputedStyle() { overlay_play_button_->ClearComputedStyle(); }
void TestKeepEventInNode() {
MouseEventInit mouse_initializer;
mouse_initializer.setView(GetDocument().domWindow());
mouse_initializer.setButton(1);
MouseEvent* mouse_event =
MouseEvent::Create(nullptr, EventTypeNames::click, mouse_initializer);
overlay_play_button_->KeepEventInNode(*mouse_event);
}
private: private:
bool SVGElementHasDisplayValue() { bool SVGElementHasDisplayValue() {
return GetElementById("jump")->InlineStyle()->HasProperty( return GetElementById("jump")->InlineStyle()->HasProperty(
...@@ -54,6 +85,7 @@ class MediaControlOverlayPlayButtonElementTest : public PageTestBase { ...@@ -54,6 +85,7 @@ class MediaControlOverlayPlayButtonElementTest : public PageTestBase {
return GetDocument().body()->getElementById(id); return GetDocument().body()->getElementById(id);
} }
Persistent<MediaControlOverlayPlayButtonElement> overlay_play_button_;
Persistent<MediaControlOverlayPlayButtonElement::AnimatedArrow> Persistent<MediaControlOverlayPlayButtonElement::AnimatedArrow>
arrow_element_; arrow_element_;
}; };
...@@ -79,4 +111,16 @@ TEST_F(MediaControlOverlayPlayButtonElementTest, ShowIncrementsCounter) { ...@@ -79,4 +111,16 @@ TEST_F(MediaControlOverlayPlayButtonElementTest, ShowIncrementsCounter) {
ExpectPresentAndShown(); ExpectPresentAndShown();
} }
TEST_F(MediaControlOverlayPlayButtonElementTest,
KeepEventInNodeDoesntCrashWithoutInternalButton) {
RemoveInternalButton();
TestKeepEventInNode();
}
TEST_F(MediaControlOverlayPlayButtonElementTest,
KeepEventInNodeDoesntCrashWithoutComputedStyle) {
RemoveComputedStyle();
TestKeepEventInNode();
}
} // namespace blink } // namespace blink
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