Clean up double Attach in media controls delegate/listener |
|
Issue descriptionMediaControlsOrientationLockDelegate and MediaControlsMediaEventListener both call Attach in their constructor if GetMediaElement().isConnected(). But this causes Attach to be called twice, since it is always called a second time by MediaControlsImpl::InsertedInto when the media controls are added to the document, which always happens after those constructors are called. They should stop calling Attach in their constructors. If MediaControlsImpl is later refactored to construct them lazily, then MediaControlsImpl should just call Attach after constructing them if the media controls are already connected.
,
Apr 26 2017
Apparently, Detach() can also either be called twice or without Attach() being called. See the failure of browse.news.reddit telemetry_perf_unittests for my CL (https://codereview.chromium.org/2782373002, PS#14) here: https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/430271 Stack trace FTR: 5564:3704:0425/211556.848:FATAL:mediacontrolsmediaeventlistener.cpp(96)] Check failed: -1 != remote_playback_availability_callback_id_ (-1 vs. -1) Backtrace: base::debug::StackTrace::StackTrace [0x6919E517+55] base::debug::StackTrace::StackTrace [0x691A04BA+10] blink::MediaControlsMediaEventListener::Detach [0x6A278BB3+343] blink::MediaControlsImpl::RemovedFrom [0x6A277C35+114] blink::ContainerNode::NotifyNodeRemoved [0x6AE561D2+88] blink::ContainerNode::NotifyNodeRemoved [0x6AE561E3+105] blink::ContainerNode::RemoveChild [0x6AE5709F+212] blink::Node::removeChild [0x6AEA5B8D+33] blink::V8Element::removeAttributeNodeMethodCallback [0x6ACB8906+211] blink::V8Node::removeChildMethodCallback [0x6ACB895F+33] v8::internal::FunctionCallbackArguments::Call [0x6895024D+157] v8::internal::Isolate::typed_array_prototype [0x689F0BEC+1788] v8::internal::Builtin_HandleApiCallAsFunction [0x689F153B+587] v8::internal::Builtin_HandleApiCall [0x689F1218+184] |
|
►
Sign in to add a comment |
|
Comment 1 by mlamouri@chromium.org
, Apr 25 2017