Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(21)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10700071: browser: Remove fullscreen functions and have callers call FullscreenController directly. (Closed)

Created:
8 years, 5 months ago by tfarina
Modified:
8 years, 5 months ago
CC:
chromium-reviews, robertshield, scheib+watch_chromium.org, kkania
Visibility:
Public.

Description

browser: Remove fullscreen functions and have callers call FullscreenController directly. NOTE: This was a TODO for @koz. R=ben@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=145685

Patch Set 1 : #

Total comments: 10

Patch Set 2 : modify to use the new accessor #

Patch Set 3 : ToggleFullscreenMode and IsFullscreenForTabOrPending #

Patch Set 4 : they are friends again #

Patch Set 5 : includes #

Patch Set 6 : more includes #

Total comments: 2

Patch Set 7 : one more include #

Patch Set 8 : chrome::ToggleFullscreenMode #

Patch Set 9 : include browser_commands.h #

Patch Set 10 : fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+64 lines, -67 lines) Patch
M chrome/browser/automation/testing_automation_provider.cc View 1 2 8 chunks +12 lines, -10 lines 0 comments Download
M chrome/browser/chromeos/login/screen_locker.cc View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/browser.h View 1 2 3 5 chunks +4 lines, -12 lines 0 comments Download
M chrome/browser/ui/browser.cc View 1 2 3 chunks +0 lines, -18 lines 0 comments Download
M chrome/browser/ui/browser_command_controller.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/browser_commands.h View 1 2 3 4 5 6 7 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/browser_commands.cc View 1 2 3 4 5 6 7 2 chunks +5 lines, -0 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_cocoa.mm View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/browser_window_controller.mm View 1 2 3 4 2 chunks +3 lines, -1 line 0 comments Download
M chrome/browser/ui/cocoa/browser_window_controller_private.mm View 1 2 3 4 3 chunks +5 lines, -2 lines 0 comments Download
M chrome/browser/ui/cocoa/fullscreen_exit_bubble_controller.mm View 1 2 3 4 2 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/ui/extensions/application_launch.cc View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/fullscreen/fullscreen_controller_browsertest.cc View 1 2 3 4 5 6 7 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/fullscreen/fullscreen_controller_test.cc View 1 2 3 4 5 6 7 4 chunks +14 lines, -13 lines 0 comments Download
M chrome/browser/ui/fullscreen/fullscreen_exit_bubble.cc View 1 2 3 4 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/ui/startup/startup_browser_creator_impl.cc View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -1 line 0 comments Download
M chrome/browser/ui/views/frame/browser_view_browsertest.cc View 1 2 3 4 5 6 7 8 9 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 7 (0 generated)
tfarina
8 years, 5 months ago (2012-07-02 23:22:02 UTC) #1
scheib
lgtm http://codereview.chromium.org/10700071/diff/10001/chrome/browser/ui/browser.cc File chrome/browser/ui/browser.cc (right): http://codereview.chromium.org/10700071/diff/10001/chrome/browser/ui/browser.cc#newcode751 chrome/browser/ui/browser.cc:751: fullscreen_controller_->ToggleFullscreenMode(); Might as well remove all wrappers around ...
8 years, 5 months ago (2012-07-03 17:08:49 UTC) #2
tfarina
http://codereview.chromium.org/10700071/diff/10001/chrome/browser/ui/browser.cc File chrome/browser/ui/browser.cc (right): http://codereview.chromium.org/10700071/diff/10001/chrome/browser/ui/browser.cc#newcode751 chrome/browser/ui/browser.cc:751: fullscreen_controller_->ToggleFullscreenMode(); On 2012/07/03 17:08:49, scheib wrote: > Might as ...
8 years, 5 months ago (2012-07-03 18:51:22 UTC) #3
Ben Goodger (Google)
http://codereview.chromium.org/10700071/diff/1018/chrome/browser/ui/cocoa/browser_window_cocoa.mm File chrome/browser/ui/cocoa/browser_window_cocoa.mm (right): http://codereview.chromium.org/10700071/diff/1018/chrome/browser/ui/cocoa/browser_window_cocoa.mm#newcode129 chrome/browser/ui/cocoa/browser_window_cocoa.mm:129: browser_->fullscreen_controller()->ToggleFullscreenMode(); Since this seems the most common use case, ...
8 years, 5 months ago (2012-07-03 19:51:35 UTC) #4
scheib
lgtm Thanks.
8 years, 5 months ago (2012-07-03 19:53:20 UTC) #5
tfarina
http://codereview.chromium.org/10700071/diff/1018/chrome/browser/ui/cocoa/browser_window_cocoa.mm File chrome/browser/ui/cocoa/browser_window_cocoa.mm (right): http://codereview.chromium.org/10700071/diff/1018/chrome/browser/ui/cocoa/browser_window_cocoa.mm#newcode129 chrome/browser/ui/cocoa/browser_window_cocoa.mm:129: browser_->fullscreen_controller()->ToggleFullscreenMode(); On 2012/07/03 19:51:35, Ben Goodger (Google) wrote: > ...
8 years, 5 months ago (2012-07-03 22:38:38 UTC) #6
Ben Goodger (Google)
8 years, 5 months ago (2012-07-09 15:27:34 UTC) #7 
lgtm

Powered by Google App Engine
This is Rietveld 408576698