New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 643418 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Refactor the code structure for Fullscreen Toolbar

Project Member Reported by spqc...@chromium.org, Sep 1 2016

Issue description

The code design that involves the fullscreen toolbar layouts is currently very messy, and that's causing lots of problems.

Rewrite it so that it functions more similarly to BrowserWindowLayout. 
 
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 31 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38

commit 32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38
Author: spqchan <spqchan@chromium.org>
Date: Mon Oct 31 20:15:01 2016

[Mac] Refactor the Fullscreen Toolbar Visibility Locks

No intended behavioral changes.
Moved the visibility lock code from BrowserWindowController into a new
class.
Renamed the methods so that it's more consistent

BUG= 643418 

Review-Url: https://codereview.chromium.org/2459653005
Cr-Commit-Position: refs/heads/master@{#428781}

[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_controller.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/bookmarks/bookmark_bubble_observer_cocoa.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/bookmarks/bookmark_editor_base_controller.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/browser_window_controller.h
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/browser_window_controller_private.h
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/browser_window_controller_private.mm
[add] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_visibility_lock_controller.h
[add] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_visibility_lock_controller.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/fullscreen_toolbar_controller.h
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/fullscreen_toolbar_controller.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/global_error_bubble_controller.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
[modify] https://crrev.com/32c3cd6f3babfda3bac5c8f5dbae07c07bf9aa38/chrome/browser/ui/cocoa/profiles/avatar_base_controller.mm

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c5b671d18c00040c5313257a619a197d4a7daaa

commit 3c5b671d18c00040c5313257a619a197d4a7daaa
Author: spqchan <spqchan@chromium.org>
Date: Fri Nov 04 19:35:56 2016

[Mac] Move the fullscreen toolbar style code into FullscreenToolbarController

- No intended behavioral changes.
- Renamed "FullscreenSlidingStyle" to "FullscreenToolbarStyle" and moved the logic
 into FullscreenToolbarController

BUG= 643418 

Review-Url: https://codereview.chromium.org/2467833003
Cr-Commit-Position: refs/heads/master@{#429979}

[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/browser/exclusive_access_controller_views.mm
[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/browser_window_controller.h
[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm
[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/browser_window_controller_private.h
[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/browser_window_controller_private.mm
[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/browser_window_layout.h
[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/browser_window_layout.mm
[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/browser_window_layout_unittest.mm
[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/fullscreen_toolbar_controller.h
[modify] https://crrev.com/3c5b671d18c00040c5313257a619a197d4a7daaa/chrome/browser/ui/cocoa/fullscreen_toolbar_controller.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69c6fdec22b2e94128f6ad016ff7b496b2e6704a

commit 69c6fdec22b2e94128f6ad016ff7b496b2e6704a
Author: spqchan <spqchan@chromium.org>
Date: Thu Nov 10 00:31:07 2016

[Mac] Refactor Immersive Fullscreen Logic

- Move immersive fullscreen logic from FullscreenToolbarController to its
own class.
- Fixed a bug where the menubar doesn't appear properly for immersive fullscreen

BUG= 643418 

Review-Url: https://codereview.chromium.org/2484973004
Cr-Commit-Position: refs/heads/master@{#431099}

[modify] https://crrev.com/69c6fdec22b2e94128f6ad016ff7b496b2e6704a/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/69c6fdec22b2e94128f6ad016ff7b496b2e6704a/chrome/browser/ui/cocoa/browser_window_controller_private.mm
[modify] https://crrev.com/69c6fdec22b2e94128f6ad016ff7b496b2e6704a/chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_mouse_tracker.mm
[add] https://crrev.com/69c6fdec22b2e94128f6ad016ff7b496b2e6704a/chrome/browser/ui/cocoa/fullscreen/immersive_fullscreen_controller.h
[add] https://crrev.com/69c6fdec22b2e94128f6ad016ff7b496b2e6704a/chrome/browser/ui/cocoa/fullscreen/immersive_fullscreen_controller.mm
[modify] https://crrev.com/69c6fdec22b2e94128f6ad016ff7b496b2e6704a/chrome/browser/ui/cocoa/fullscreen_toolbar_controller.h
[modify] https://crrev.com/69c6fdec22b2e94128f6ad016ff7b496b2e6704a/chrome/browser/ui/cocoa/fullscreen_toolbar_controller.mm
[modify] https://crrev.com/69c6fdec22b2e94128f6ad016ff7b496b2e6704a/ui/base/cocoa/tracking_area.h
[modify] https://crrev.com/69c6fdec22b2e94128f6ad016ff7b496b2e6704a/ui/base/cocoa/tracking_area.mm

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9dd1729a24326039f5b4cd36d6697e97af0d2938

commit 9dd1729a24326039f5b4cd36d6697e97af0d2938
Author: spqchan <spqchan@chromium.org>
Date: Wed Nov 16 22:22:16 2016

[Mac] FullscreenToolbarController Refactor

No intentional behavioral changes

- Rearranged the functions so that the order matches the header file
- Removed unnecessary imports
- Renamed functions

BUG= 643418 

Review-Url: https://codereview.chromium.org/2505653003
Cr-Commit-Position: refs/heads/master@{#432644}

[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/browser/exclusive_access_controller_views.mm
[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/browser_window_controller_browsertest.mm
[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/browser_window_controller_private.h
[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/browser_window_controller_private.mm
[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/browser_window_layout.h
[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/fullscreen/fullscreen_menubar_tracker.mm
[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_animation_controller.mm
[rename] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_controller.h
[rename] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_controller.mm
[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_mouse_tracker.mm
[modify] https://crrev.com/9dd1729a24326039f5b4cd36d6697e97af0d2938/chrome/browser/ui/cocoa/fullscreen/fullscreen_toolbar_visibility_lock_controller.mm

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 28 2016

Status: Fixed (was: Assigned)

Sign in to add a comment