Make mediaPlayerRequestFullscreen() user gesture bypass explicit

Source/core/html/HTMLMediaElement.cpp

 /*
  * Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 2831 matching lines @@
 
     if (m_player->paused())
         pause();
     else
         playInternal();
 }
 
 void HTMLMediaElement::mediaPlayerRequestFullscreen()
 {
     WTF_LOG(Media, "HTMLMediaElement::mediaPlayerRequestFullscreen");
+
+    // The player is responsible for only invoking this callback in response to
+    // user interaction or when it is technically required to play the video.
+    UserGestureIndicator gestureIndicator(DefinitelyProcessingNewUserGesture);

[qinmin, 2014/03/24 06:32:54]

will the gesture be consumed by FullscreenElementStack? Otherwise, it will
invoke some other functions that require user gesture.

[philipj_slow, 2014/03/24 15:18:23]

The UserGestureIndicator is only active while it's on the stack, see its
constructor and destructor. No events are dispatches synchronously in
FullscreenElementStack::requestFullScreenForElement so AFAICT there's no way for
scripts to run and abuse the user gesture for something else.

As for consuming gestures, it seems like that's only done from Chromium and
internally in UserGestureIndicator, so I don't think there's anything I should
do with that.

[acolwell GONE FROM CHROMIUM, 2014/03/25 22:14:43]

How about we just move the settings code from FullscreenElementStack.cpp to here
so the code looks something like?

if (!document()->settings()->mediaFullscreenRequiresUserGesture()) {
  UserGestureIndicator gestureIndicator(DefinitelyProcessingNewUserGesture);
  enterFullscreen();
  return;
}

enterFullscreen();

It is a slight duplication in code, but it puts all the logic in one place.

[philipj_slow, 2014/03/26 07:48:17]

Sorry, I didn't communicate my intentions very clearly. I mean to remove the
setting entirely so that the code in
FullscreenElementStack::requestFullScreenForElement becomes just:

if (!UserGestureIndicator::processingUserGesture())
    break;

The reason I didn't do it in this CL is because there's a tests for the setting
that would start failing, and I'd rather fix that when removing the setting
entirely. Here are the two CLs that would finish the job:

https://codereview.chromium.org/212133003/
https://codereview.chromium.org/212323003/

However, if you want to keep the setting, I agree that moving it as suggested
would be an improvement, since it limits the exemption to just
mediaPlayerRequestFullscreen(). Let me know what you prefer, even if it's
leaving things as they are.

[acolwell GONE FROM CHROMIUM, 2014/03/26 17:39:43]

Ok. Given the other 2 CLs, I'm fine with doing deferring the
FullscreenElementStack change until https://codereview.chromium.org/212323003/

+
     enterFullscreen();
 }
 
 void HTMLMediaElement::mediaPlayerRequestSeek(double time)
 {
     // The player is the source of this seek request.
     if (m_mediaController) {
         m_mediaController->setCurrentTime(time, IGNORE_EXCEPTION);
         return;
     }
@@ skipping 797 matching lines @@
 {
     m_mediaSource->setWebMediaSourceAndOpen(adoptPtr(webMediaSource));
 }
 
 bool HTMLMediaElement::isInteractiveContent() const
 {
     return fastHasAttribute(controlsAttr);
 }
 
 }