Fix the conditions when fullscreen detector listeners are registered

Fix the conditions when fullscreen detector listeners are registered

Previously, we rely on the DOMNode[Inserted/Removed]IntoDocument events
to attach/detach the detector. However this won't work if the video
element is created from the parsing process. This CL fixes the issue by
overriding insertedInto() and removedFrom() callbacks, which always
works regardless of how the video element is inserted into the document.

BUG=703210
Review-Url: https://codereview.chromium.org/2758233003
Cr-Commit-Position: refs/heads/master@{#458147}
Committed: https://chromium.googlesource.com/chromium/src/+/d9d39e2308b617bad51c86f9703828a2fbef95d8

[Zhiqiang Zhang (Slow), 2017-03-20 16:49:04]

zqzhang@chromium.org changed reviewers:
+ mlamouri@chromium.org

[Zhiqiang Zhang (Slow), 2017-03-20 16:49:05]

PTAL

[mlamouri (slow - plz ping), 2017-03-20 17:05:25]

lgtm with comment addressed

https://codereview.chromium.org/2758233003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLVideoElement.cpp (right):

https://codereview.chromium.org/2758233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLVideoElement.cpp:93: if
(insertionPoint->inActiveDocument() && m_customControlsFullscreenDetector)
OOC, why are you adding this? I did not really understood why HTMLMediaELement
had this check. MediaControls doesn't have it.

[Zhiqiang Zhang (Slow), 2017-03-20 17:14:22]

https://codereview.chromium.org/2758233003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLVideoElement.cpp (right):

https://codereview.chromium.org/2758233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLVideoElement.cpp:93: if
(insertionPoint->inActiveDocument() && m_customControlsFullscreenDetector)
On 2017/03/20 at 17:05:25, mlamouri wrote:
> OOC, why are you adding this? I did not really understood why HTMLMediaELement
had this check. MediaControls doesn't have it.

The check is to see if the node was connected to an active document before
removing. I think we can remove it as removing unregistered listeners is no-op.

[Zhiqiang Zhang (Slow), 2017-03-20 17:23:55]

zqzhang@chromium.org changed reviewers:
+ skobes@chromium.org

[Zhiqiang Zhang (Slow), 2017-03-20 17:23:55]

+skobes for rubber-stamping HTMLVideoElement.*

[skobes, 2017-03-20 17:33:00]

rs lgtm

[Zhiqiang Zhang (Slow), 2017-03-20 17:33:39]

The CQ bit was checked by zqzhang@chromium.org

[Zhiqiang Zhang (Slow), 2017-03-20 17:33:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2758233003/#ps20001
(title: "addressed mlamouri's comments")

[commit-bot: I haz the power, 2017-03-20 17:33:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-20 19:17:21]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490031219864900,
"parent_rev": "7f94243892116f4266e8f532db6977cd5f540c2d", "commit_rev":
"d9d39e2308b617bad51c86f9703828a2fbef95d8"}

[commit-bot: I haz the power, 2017-03-20 19:18:05]

Description was changed from

==========
Fix the conditions when fullscreen detector listeners are registered

Previously, we rely on the DOMNode[Inserted/Removed]IntoDocument events
to attach/detach the detector. However this won't work if the video
element is created from the parsing process. This CL fixes the issue by
overriding insertedInto() and removedFrom() callbacks, which always
works regardless of how the video element is inserted into the document.

BUG=703210
==========

to

==========
Fix the conditions when fullscreen detector listeners are registered

Previously, we rely on the DOMNode[Inserted/Removed]IntoDocument events
to attach/detach the detector. However this won't work if the video
element is created from the parsing process. This CL fixes the issue by
overriding insertedInto() and removedFrom() callbacks, which always
works regardless of how the video element is inserted into the document.

BUG=703210

Review-Url: https://codereview.chromium.org/2758233003
Cr-Commit-Position: refs/heads/master@{#458147}
Committed:
https://chromium.googlesource.com/chromium/src/+/d9d39e2308b617bad51c86f97038...
==========

[commit-bot: I haz the power, 2017-03-20 19:18:08]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/d9d39e2308b617bad51c86f97038...