Remove document().page() null checks in MediaControls

Remove document().page() null checks in MediaControls

These were needed in WebKit when the render theme was accessed via the
page, but that is no longer the case, these null checks are the only
bits in MediaControls that access document().page() at all.

It's possible that some code deeper down assumes that the page is not
null and will now begin to crash, but a quick scan of Source/core/
didn't reveal anything obvious.

BUG=341813
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168855

[acolwell GONE FROM CHROMIUM, 2014-03-08 20:14:31]

Could we potentially get a crash if an HTMLMediaElement was pulled out of an
iframe, a reference is maintained by JS, the iframe is destroyed, and the
something is done to trigger a controls update? It's not clear to me whether the
element would have a valid document in that case. I'm thinking of cases similar
to what scherkus@ fixed with
http://src.chromium.org/viewvc/blink?view=revision&revision=167935

[eseidel, 2014-03-08 20:37:00]

These days we ask Document::isActive() since that requires the document to be in
a frame and that frame be in a page to be true.

But I'm all for removing these sorts of checks and fixing this sort of null
check at a higher level by not entering this code when the document is not
active.

I recently regressed a bunch of these similar checks in the fullscreen code by
removing one of these and had to add it back.  But the real fix there is to fix
fullscreen code not to be entered when teh document is not active.

[eseidel, 2014-03-08 20:39:04]

https://codereview.chromium.org/190463002/diff/1/Source/core/html/shadow/Medi...
File Source/core/html/shadow/MediaControls.cpp (left):

https://codereview.chromium.org/190463002/diff/1/Source/core/html/shadow/Medi...
Source/core/html/shadow/MediaControls.cpp:59: PassRefPtr<MediaControls>
MediaControls::create(Document& document)
One way to test these methods is to do:

<iframe id="iframe" src="about:blank">
<script>
var detachedDoc = iframe.contentDocument;
iframe.src ="whatever";
detachedDoc.tryToGrabAtMediaControlsStuff();
</script>

You may want to see if you can enter these codepaths that way before removing
these nullchecks.

[philipj_slow, 2014-03-09 17:15:28]

I've added a test that triggers all the removed code paths except
MediaControls::startHideFullscreenControlsTimer(), verified by replacing the
null checks by ASSERT(document().page()) and seeing a crash, one function at a
time. In other words, it's not that document().page() can't be null, it's that
it doesn't matter. The media controls are mostly just manipulating the DOM in
ways which scripts can already do in detached documents, after all. After
following the code in startHideFullscreenControlsTimer() I'm not worried about
that case either, it just ends up in MediaControlPanelElement::makeTransparent()
which sets some style. (After a timeout, so the null check would be misplaced if
it were needed.)

I wanted to write a test that actually plays video in a frameless document, but
discovered that doesn't work! I've submitted a test case to web-platform-tests,
and since my goal is for Blink to pass all tests in wpt I'll eventually get to
investigating: https://github.com/w3c/web-platform-tests/pull/736

[eseidel, 2014-03-10 19:13:09]

lgtm

[commit-bot: I haz the power, 2014-03-10 19:13:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/philipj@opera.com/190463002/20001

[commit-bot: I haz the power, 2014-03-10 19:20:04]

Change committed as 168855