Add policy for fullscreen mode; disallow fullscreen in public sessions

Add policy for fullscreen mode; disallow fullscreen in public sessions

This CL adds a user policy that determines whether fullscreen mode is
allowed and uses it to disallow fullscreen mode in public sessions.

BUG=275405
TEST=New browser tests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=219482

[bartfab (slow), 2013-08-19 12:42:34]

Hi Mattias,

Could you take a look at this CL?

Hi Scott,

Could you take a look at chrome/browser/ui/*?

[Mattias Nissler (ping if slow), 2013-08-19 13:29:49]

Code LGTM, but shouldn't we disable all the UI elements that allow the user to
toggle fullscreen as well?

https://codereview.chromium.org/22986010/diff/1/chrome/app/policy/policy_temp...
File chrome/app/policy/policy_templates.json (right):

https://codereview.chromium.org/22986010/diff/1/chrome/app/policy/policy_temp...
chrome/app/policy/policy_templates.json:4214: 'desc': '''Allow fullscreen mode.
Can you add a quick summary of what fullscreen mode is?

[sky, 2013-08-19 15:50:47]

Do you need to update the enabled state of IDC_FULLSCREEN appropriately too?

[scheib, 2013-08-19 16:24:07]

https://codereview.chromium.org/22986010/diff/1/chrome/app/policy/policy_temp...
File chrome/app/policy/policy_templates.json (right):

https://codereview.chromium.org/22986010/diff/1/chrome/app/policy/policy_temp...
chrome/app/policy/policy_templates.json:4216: If this policy is set to true or
not configured, the user and apps/extensions with appropriate permissions can
enter fullscreen mode.
This patch is not limiting apps from fullscreen. If that is what you intend to
do you will need to modify ShellWindow / NativeAppWindow* ::*Fullscreen(). Care
must be taken on Mac (maybe ChromeOS) as the window can be made fullscreen
outside of the ShellWindow::Fullscree() method (via titlebar controls -- there's
an outstanding issue to make those work together correctly).

[bartfab (slow), 2013-08-22 20:21:13]

Mattias, Scott, Vincent,

Could you take another look at the CL?

https://chromiumcodereview.appspot.com/22986010/diff/1/chrome/app/policy/poli...
File chrome/app/policy/policy_templates.json (right):

https://chromiumcodereview.appspot.com/22986010/diff/1/chrome/app/policy/poli...
chrome/app/policy/policy_templates.json:4214: 'desc': '''Allow fullscreen mode.
On 2013/08/19 13:29:50, Mattias Nissler wrote:
> Can you add a quick summary of what fullscreen mode is?

Done.

https://chromiumcodereview.appspot.com/22986010/diff/1/chrome/app/policy/poli...
chrome/app/policy/policy_templates.json:4216: If this policy is set to true or
not configured, the user and apps/extensions with appropriate permissions can
enter fullscreen mode.
On 2013/08/19 16:24:08, scheib wrote:
> This patch is not limiting apps from fullscreen. If that is what you intend to
> do you will need to modify ShellWindow / NativeAppWindow* ::*Fullscreen().
Care
> must be taken on Mac (maybe ChromeOS) as the window can be made fullscreen
> outside of the ShellWindow::Fullscree() method (via titlebar controls --
there's
> an outstanding issue to make those work together correctly).

Thanks for the pointers.

Re apps: I extended the CL to prevent apps from using fullscreen as well and
manually tested that the blocking works.

Re Chrome OS: I remember immersive mode added UI elements to the titlebar that
would go directly to fullscreen. These seem to have been removed since. Even
enabling immersive mode via a flag does not bring back the controls. Hence, it
seems that this loophole has been plugged.

Re Mac OS: I was not aware of the fullscreen button. Since the button is not
directly managed by Chrome, working around it would have been tricky. As we do
not need this new policy on Mac right now, I excluded Mac from the list of
platforms that the policy is supported on.

[sky, 2013-08-22 21:33:55]

I only looked at c/c and c/b/u (not c/b/u/f, I'm assuming scheib will do that).

https://chromiumcodereview.appspot.com/22986010/diff/11001/chrome/browser/ui/...
File chrome/browser/ui/browser_command_controller.cc (right):

https://chromiumcodereview.appspot.com/22986010/diff/11001/chrome/browser/ui/...
chrome/browser/ui/browser_command_controller.cc:65: FULLSCREEN_DISABLED,
Am I the only one that finds this name totally misleading?

[scheib, 2013-08-22 23:57:04]

> Re Mac OS: I was not aware of the fullscreen button. Since the button is not
> directly managed by Chrome, working around it would have been tricky. As we do
> not need this new policy on Mac right now, I excluded Mac from the list of
> platforms that the policy is supported on.

I don't think you should disable this feature on Mac.

First, even with no additional code I think the behavior on Mac may comply with
your intent: Even when fullscreen, the system menubar is available at all times
and drops down over web & apps. The dock is also always available by mousing
down to the bottom. Chrome will still be blocked from being in its fullscreen
mode.

Second, I think you can do a bit more work and disable the fullscreen option.
Search for 
NSWindowCollectionBehaviorFullScreen and prevent it from being added in 
chromium/src/chrome/browser/ui/cocoa/browser_window_controller.mm
chromium/src/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm
(possibly in other locations, there aren't many to check)

This will disable the window's ability to fullscreen at the window manager
level. See
https://developer.apple.com/library/mac/documentation/General/Conceptual/MOSX...
and setCollectionBehavior.

[scheib, 2013-08-22 23:57:26]

https://chromiumcodereview.appspot.com/22986010/diff/11001/chrome/app/policy/...
File chrome/app/policy/policy_templates.json (right):

https://chromiumcodereview.appspot.com/22986010/diff/11001/chrome/app/policy/...
chrome/app/policy/policy_templates.json:4261: If this policy is set to false,
neither the user nor any apps can enter fullscreen mode.''',
user, apps, and extensions (presuming extensions are blocked as well, from the
implementation I expect them to be).

(see http://developer.chrome.com/extensions/windows.html state)

https://chromiumcodereview.appspot.com/22986010/diff/11001/chrome/browser/ui/...
File chrome/browser/ui/browser_command_controller.cc (right):

https://chromiumcodereview.appspot.com/22986010/diff/11001/chrome/browser/ui/...
chrome/browser/ui/browser_command_controller.cc:65: FULLSCREEN_DISABLED,
On 2013/08/22 21:33:55, sky wrote:
> Am I the only one that finds this name totally misleading?

WINDOW_STATE_NORMAL,
WINDOW_STATE_FULLSCREEN,
WINDOW_STATE_METRO_SNAP,

https://chromiumcodereview.appspot.com/22986010/diff/11001/chrome/browser/ui/...
File chrome/browser/ui/fullscreen/fullscreen_controller.cc (right):

https://chromiumcodereview.appspot.com/22986010/diff/11001/chrome/browser/ui/...
chrome/browser/ui/fullscreen/fullscreen_controller.cc:559: // Do not enter
fullscreen mode if disallowed by pref.
Have you tested this in combination with kiosk mode? Please add comments here
explaining now they mix (or don't mix). Add explanation elsewhere also (e.g.
when exposing preferences) if they don't mix.

[sky, 2013-08-23 00:00:37]

https://chromiumcodereview.appspot.com/22986010/diff/11001/chrome/browser/ui/...
File chrome/browser/ui/browser_command_controller.cc (right):

https://chromiumcodereview.appspot.com/22986010/diff/11001/chrome/browser/ui/...
chrome/browser/ui/browser_command_controller.cc:65: FULLSCREEN_DISABLED,
On 2013/08/22 23:57:26, scheib wrote:
> On 2013/08/22 21:33:55, sky wrote:
> > Am I the only one that finds this name totally misleading?
> 
> WINDOW_STATE_NORMAL,
> WINDOW_STATE_FULLSCREEN,
> WINDOW_STATE_METRO_SNAP,
> 

Much better, but NORMAL is a bit ambiguous here, I would go with
WINDOW_STATE_NOT_FULLSCREEN.

I wasn't clear on why I didn't like the current name. Currently
FULLSCREEN_DISABLED means 'not in fullscreen', which is very confusing.

[bartfab (slow), 2013-08-23 11:30:21]

On 2013/08/22 23:57:04, scheib wrote:
> > Re Mac OS: I was not aware of the fullscreen button. Since the button is not
> > directly managed by Chrome, working around it would have been tricky. As we
do
> > not need this new policy on Mac right now, I excluded Mac from the list of
> > platforms that the policy is supported on.
> 
> I don't think you should disable this feature on Mac.
> 
> First, even with no additional code I think the behavior on Mac may comply
with
> your intent: Even when fullscreen, the system menubar is available at all
times
> and drops down over web & apps. The dock is also always available by mousing
> down to the bottom. Chrome will still be blocked from being in its fullscreen
> mode.
> 
> Second, I think you can do a bit more work and disable the fullscreen option.
> Search for 
> NSWindowCollectionBehaviorFullScreen and prevent it from being added in 
> chromium/src/chrome/browser/ui/cocoa/browser_window_controller.mm
> chromium/src/chrome/browser/ui/cocoa/apps/native_app_window_cocoa.mm
> (possibly in other locations, there aren't many to check)
> 
> This will disable the window's ability to fullscreen at the window manager
> level. See
>
https://developer.apple.com/library/mac/documentation/General/Conceptual/MOSX...
> and setCollectionBehavior.

The intent of this policy is to ensure that system UI such as the shared menu
bar on Mac or the dock/task bar cannot be hidden from inside Chrome. Hence,
fullscreen mode would need to be disabled for this policy to reach its intenten
on Mac as well.

Simply preventing the button from being added is not enough. The button would
need to be added/removed dynamically as fullscreen is allowed/disallowed. Also,
the button is used to toggle between normal/fullscreen/presentation mode. If
fullscreen becomes disallowed but the user currently is in fullscreen or
presentation mode, the button would have to stay visible until the user clicks
it to get back to normal mode. Only at that point should the button be hidden.

I am sure that it is possible to implement this on Mac. But as we currently have
no use case for it and I do not have access to a Mac, I think that a policy for
CrOS/Linux/Win is a good solution. We can always extend it Mac when the need
arises.

[bartfab (slow), 2013-08-23 11:54:21]

https://codereview.chromium.org/22986010/diff/11001/chrome/app/policy/policy_...
File chrome/app/policy/policy_templates.json (right):

https://codereview.chromium.org/22986010/diff/11001/chrome/app/policy/policy_...
chrome/app/policy/policy_templates.json:4261: If this policy is set to false,
neither the user nor any apps can enter fullscreen mode.''',
On 2013/08/22 23:57:26, scheib wrote:
> user, apps, and extensions (presuming extensions are blocked as well, from the
> implementation I expect them to be).
> 
> (see http://developer.chrome.com/extensions/windows.html state)

Done.

I verified that "chrome.windows.update();" is prevented from switching a window
into full screen and updated the documentation.

https://codereview.chromium.org/22986010/diff/11001/chrome/browser/ui/browser...
File chrome/browser/ui/browser_command_controller.cc (right):

https://codereview.chromium.org/22986010/diff/11001/chrome/browser/ui/browser...
chrome/browser/ui/browser_command_controller.cc:65: FULLSCREEN_DISABLED,
On 2013/08/23 00:00:37, sky wrote:
> On 2013/08/22 23:57:26, scheib wrote:
> > On 2013/08/22 21:33:55, sky wrote:
> > > Am I the only one that finds this name totally misleading?
> > 
> > WINDOW_STATE_NORMAL,
> > WINDOW_STATE_FULLSCREEN,
> > WINDOW_STATE_METRO_SNAP,
> > 
> 
> Much better, but NORMAL is a bit ambiguous here, I would go with
> WINDOW_STATE_NOT_FULLSCREEN.
> 
> I wasn't clear on why I didn't like the current name. Currently
> FULLSCREEN_DISABLED means 'not in fullscreen', which is very confusing.

Done.

https://codereview.chromium.org/22986010/diff/11001/chrome/browser/ui/fullscr...
File chrome/browser/ui/fullscreen/fullscreen_controller.cc (right):

https://codereview.chromium.org/22986010/diff/11001/chrome/browser/ui/fullscr...
chrome/browser/ui/fullscreen/fullscreen_controller.cc:559: // Do not enter
fullscreen mode if disallowed by pref.
On 2013/08/22 23:57:26, scheib wrote:
> Have you tested this in combination with kiosk mode? Please add comments here
> explaining now they mix (or don't mix). Add explanation elsewhere also (e.g.
> when exposing preferences) if they don't mix.

Added documentation here, in pref_names.cc and policy_templates.json.

On desktop platforms, disabling fullscreen makes kiosk mode unavailable. On
Chrome OS, disabling fullscreen has no impact on kiosk mode. (This is because
desktop kiosk mode and Chrome OS kiosk mode are entirely unrelated features that
unfortunately happen to share the same name.)

[sky, 2013-08-23 16:39:27]

LGTM

[scheib, 2013-08-23 17:27:07]

lgtm

[commit-bot: I haz the power, 2013-08-25 12:42:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bartfab@chromium.org/22986010/44001

[commit-bot: I haz the power, 2013-08-25 15:48:36]

Change committed as 219482