Rename fullscreen_exit_bubble_* to exclusive_access_bubble_*

Fullscreen exit bubble is actually used to control both mouse lock and fullscreen. Addition of keyboard lock would make it the third behavior requiring some clean-up. This is part 1 of the refactor and contains just renames.

Also the difference from the original CL is that I have reverted rename of fullscreen_controller in lieu of doing it when I actually move the functionality and using exclusive_access_* instead of lock_exit.

BUG=166928
Committed: https://crrev.com/49dd49512c7ee768231b5cca8a5ad475273a012b
Cr-Commit-Position: refs/heads/master@{#308231}

[Sriram, 2014-12-11 20:46:54]

sriramsr@chromium.org changed reviewers:
+ miu@chromium.org, scheib@chromium.org, sky@chromium.org

[scheib, 2014-12-11 20:50:33]

lgtm, thanks for iterating on the name.

[miu, 2014-12-11 23:10:51]

lgtm % nits:

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/exclusive_access_bubble_window_controller.h
(right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/exclusive_access_bubble_window_controller.h:16: // user
how to escape fullscreen mode. The bubble only appears when a tab
This comment needs updating.  We should list the three things the user can
escape from: fullscreen, mouse lock, and keyboard lock.

Also, it's inaccurate to say "the bubble only appears when a tab requests..."
since there are other conditions as well.  Rather than enumerate all that, you
could point the reader to look at the EAB_* enum.

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
File chrome/browser/ui/fullscreen/exclusive_access_bubble_type.h (right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
chrome/browser/ui/fullscreen/exclusive_access_bubble_type.h:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
nit: Consider re-uploading with --similarity=10 to maintain this file's history
across the rename.  Meaning:

  git cl upload --similarity=10

Hit CTRL-C at the "patch set description" prompt if the git diff status output
doesn't indicate the file renames correctly, and try different values for
similarity until you get the desired results.

[sky, 2014-12-12 00:00:37]

Is the long term plan to move the exclusive_access foo files out of fullscreen
and into their own directory?

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/browser.h
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/browse...
chrome/browser/ui/browser.h:501:
FRIEND_TEST_ALL_PREFIXES(ExclusiveAccessBubbleWindowControllerTest,
nit: keep these sorted.

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/exclusive_access_bubble_window_controller.mm
(right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/exclusive_access_bubble_window_controller.mm:83: if
(bubbleType_ == EAB_TYPE_FULLSCREEN_MOUSELOCK_EXIT_INSTRUCTION ||
I get why you went with EAB here, but IMO the super long
EXCLUSIVE_ACCESS_BUBBLE_TYPE_XXX is preferable. EAB_ is only meaningful to folks
that already know it.
Here's what the style guide says:
"Function names, variable names, and filenames should be descriptive; eschew
abbreviation.

Give as descriptive a name as possible, within reason. Do not worry about saving
horizontal space as it is far more important to make your code immediately
understandable by a new reader. Do not use abbreviations that are ambiguous or
unfamiliar to readers outside your project, and do not abbreviate by deleting
letters within a word."

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
File chrome/browser/ui/fullscreen/exclusive_access_bubble.h (right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
chrome/browser/ui/fullscreen/exclusive_access_bubble.h:20: class
ExclusiveAccessBubble : public gfx::AnimationDelegate {
FullscreenXXX is rather self descriptive. ExclusiveAccess isn't. All this code
needs comments to clarify what exclusive access means.

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
File chrome/browser/ui/fullscreen/exclusive_access_bubble_type.h (right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
chrome/browser/ui/fullscreen/exclusive_access_bubble_type.h:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
On 2014/12/11 23:10:50, miu wrote:
> nit: Consider re-uploading with --similarity=10 to maintain this file's
history
> across the rename.  Meaning:
> 
>   git cl upload --similarity=10
> 
> Hit CTRL-C at the "patch set description" prompt if the git diff status output
> doesn't indicate the file renames correctly, and try different values for
> similarity until you get the desired results.

Yes please.

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/exclusive_access_bubble_views.h (right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/exclusive_access_bubble_views.h:25: //
FullscreenExitBubbleViews is responsible for showing a bubble atop the
Again, update comments.

[Sriram, 2014-12-12 21:56:39]

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/browser.h
File chrome/browser/ui/browser.h (right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/browse...
chrome/browser/ui/browser.h:501:
FRIEND_TEST_ALL_PREFIXES(ExclusiveAccessBubbleWindowControllerTest,
On 2014/12/12 00:00:37, sky wrote:
> nit: keep these sorted.

Done.

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/exclusive_access_bubble_window_controller.h
(right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/exclusive_access_bubble_window_controller.h:16: // user
how to escape fullscreen mode. The bubble only appears when a tab
On 2014/12/11 23:10:50, miu wrote:
> This comment needs updating.  We should list the three things the user can
> escape from: fullscreen, mouse lock, and keyboard lock.
> 
> Also, it's inaccurate to say "the bubble only appears when a tab requests..."
> since there are other conditions as well.  Rather than enumerate all that, you
> could point the reader to look at the EAB_* enum.

Done.

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
File chrome/browser/ui/fullscreen/exclusive_access_bubble.h (right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
chrome/browser/ui/fullscreen/exclusive_access_bubble.h:20: class
ExclusiveAccessBubble : public gfx::AnimationDelegate {
Added comment to explain what and when this is used. The part of the reason for
the rename is that the class is used in multiple cases - i.e. fullscreen as well
as mouse lock (and keyboard lock in near future).

On 2014/12/12 00:00:37, sky wrote:
> FullscreenXXX is rather self descriptive. ExclusiveAccess isn't. All this code
> needs comments to clarify what exclusive access means.

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
File chrome/browser/ui/fullscreen/exclusive_access_bubble_type.h (right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
chrome/browser/ui/fullscreen/exclusive_access_bubble_type.h:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
On 2014/12/11 23:10:50, miu wrote:
> nit: Consider re-uploading with --similarity=10 to maintain this file's
history
> across the rename.  Meaning:
> 
>   git cl upload --similarity=10
> 
> Hit CTRL-C at the "patch set description" prompt if the git diff status output
> doesn't indicate the file renames correctly, and try different values for
> similarity until you get the desired results.

Done.

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/views/...
File chrome/browser/ui/views/exclusive_access_bubble_views.h (right):

https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/views/...
chrome/browser/ui/views/exclusive_access_bubble_views.h:25: //
FullscreenExitBubbleViews is responsible for showing a bubble atop the
On 2014/12/12 00:00:37, sky wrote:
> Again, update comments.

Done.

[Sriram, 2014-12-12 21:58:05]

As for the long term plan - yes. I will do that once I have the refactor done.

On 2014/12/12 00:00:37, sky wrote:
> Is the long term plan to move the exclusive_access foo files out of fullscreen
> and into their own directory?
> 
>
https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/browser.h
> File chrome/browser/ui/browser.h (right):
> 
>
https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/browse...
> chrome/browser/ui/browser.h:501:
> FRIEND_TEST_ALL_PREFIXES(ExclusiveAccessBubbleWindowControllerTest,
> nit: keep these sorted.
> 
>
https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/cocoa/...
> File chrome/browser/ui/cocoa/exclusive_access_bubble_window_controller.mm
> (right):
> 
>
https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/cocoa/...
> chrome/browser/ui/cocoa/exclusive_access_bubble_window_controller.mm:83: if
> (bubbleType_ == EAB_TYPE_FULLSCREEN_MOUSELOCK_EXIT_INSTRUCTION ||
> I get why you went with EAB here, but IMO the super long
> EXCLUSIVE_ACCESS_BUBBLE_TYPE_XXX is preferable. EAB_ is only meaningful to
folks
> that already know it.
> Here's what the style guide says:
> "Function names, variable names, and filenames should be descriptive; eschew
> abbreviation.
> 
> Give as descriptive a name as possible, within reason. Do not worry about
saving
> horizontal space as it is far more important to make your code immediately
> understandable by a new reader. Do not use abbreviations that are ambiguous or
> unfamiliar to readers outside your project, and do not abbreviate by deleting
> letters within a word."
> 
>
https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
> File chrome/browser/ui/fullscreen/exclusive_access_bubble.h (right):
> 
>
https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
> chrome/browser/ui/fullscreen/exclusive_access_bubble.h:20: class
> ExclusiveAccessBubble : public gfx::AnimationDelegate {
> FullscreenXXX is rather self descriptive. ExclusiveAccess isn't. All this code
> needs comments to clarify what exclusive access means.
> 
>
https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
> File chrome/browser/ui/fullscreen/exclusive_access_bubble_type.h (right):
> 
>
https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/fullsc...
> chrome/browser/ui/fullscreen/exclusive_access_bubble_type.h:1: // Copyright
(c)
> 2012 The Chromium Authors. All rights reserved.
> On 2014/12/11 23:10:50, miu wrote:
> > nit: Consider re-uploading with --similarity=10 to maintain this file's
> history
> > across the rename.  Meaning:
> > 
> >   git cl upload --similarity=10
> > 
> > Hit CTRL-C at the "patch set description" prompt if the git diff status
output
> > doesn't indicate the file renames correctly, and try different values for
> > similarity until you get the desired results.
> 
> Yes please.
> 
>
https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/views/...
> File chrome/browser/ui/views/exclusive_access_bubble_views.h (right):
> 
>
https://codereview.chromium.org/789403002/diff/20001/chrome/browser/ui/views/...
> chrome/browser/ui/views/exclusive_access_bubble_views.h:25: //
> FullscreenExitBubbleViews is responsible for showing a bubble atop the
> Again, update comments.

[sky, 2014-12-13 00:09:34]

LGTM

[miu, 2014-12-13 00:20:33]

Still lgtm.

BTW--With regards to the long-term plan, it might make sense to start by
renaming the fullscreen directory itself to something like exclusive_access,
rather than create a separate dir.  Not in this change, though.  ;-)

[miu, 2014-12-13 00:20:49]

The CQ bit was checked by miu@chromium.org

[commit-bot: I haz the power, 2014-12-13 00:22:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/789403002/40001

[commit-bot: I haz the power, 2014-12-13 02:54:12]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-12-13 02:54:56]

Patchset 3 (id:??) landed as
https://crrev.com/49dd49512c7ee768231b5cca8a5ad475273a012b
Cr-Commit-Position: refs/heads/master@{#308231}