Issue 10871020: Move Fullscreen stuff onto FullscreenHandler type that hangs off NWW. This is the first step in mov…

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Issue 10871020: Move Fullscreen stuff onto FullscreenHandler type that hangs off NWW. This is the first step in mov… (Closed)

Created:
8 years, 4 months ago by Ben Goodger (Google)

Modified:
8 years, 4 months ago

Reviewers:
sky

CC:
chromium-reviews, tfarina, ben+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src/

Visibility:
Public.

More Reviews

Description

Move Fullscreen stuff onto FullscreenHandler type that hangs off NWW. This is the first step in moving it to HWNDMessageHandler. Move window visibility scoping to ScopedFullscreenVisibility for safety. http://crbug.com/142962 R=sky@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=152803

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Patch Set 3 : #

Created: 8 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+302 lines, -144 lines)			Patch
M	chrome/browser/ui/views/frame/browser_view.cc	View	1	3 chunks	+2 lines, -6 lines	0 comments	Download
M	ui/views/views.gyp	View		2 chunks	+12 lines, -0 lines	0 comments	Download
M	ui/views/widget/native_widget_win.h	View		7 chunks	+2 lines, -42 lines	0 comments	Download
M	ui/views/widget/native_widget_win.cc	View		7 chunks	+13 lines, -96 lines	0 comments	Download
M	ui/views/win/DEPS	View		1 chunk	+4 lines, -0 lines	0 comments	Download
A	ui/views/win/fullscreen_handler.h	View		1 chunk	+62 lines, -0 lines	0 comments	Download
A	ui/views/win/fullscreen_handler.cc	View		1 chunk	+113 lines, -0 lines	0 comments	Download
A	ui/views/win/scoped_fullscreen_visibility.h	View		1 chunk	+41 lines, -0 lines	0 comments	Download
A	ui/views/win/scoped_fullscreen_visibility.cc	View	1 2	1 chunk	+53 lines, -0 lines	0 comments	Download

Messages

Total messages: 2 (0 generated)

Expand Messages | Collapse Messages

sky

8 years, 4 months ago (2012-08-22 19:13:47 UTC) #2

LGTM

http://codereview.chromium.org/10871020/diff/1/chrome/browser/ui/views/frame/...
File chrome/browser/ui/views/frame/browser_view.cc (right):

http://codereview.chromium.org/10871020/diff/1/chrome/browser/ui/views/frame/...
chrome/browser/ui/views/frame/browser_view.cc:112: #include
"ui/views/win/scoped_fullscreen_visibility.h"
Should this be around line 127?

http://codereview.chromium.org/10871020/diff/1/ui/views/win/scoped_fullscreen...
File ui/views/win/scoped_fullscreen_visibility.cc (right):

http://codereview.chromium.org/10871020/diff/1/ui/views/win/scoped_fullscreen...
ui/views/win/scoped_fullscreen_visibility.cc:38: ShowWindow(hwnd_, SW_SHOW);
nit: if (full_screen_windows_->empty()) {
  delete full_screen_windows_;
  full_screen_windows_ = NULL;
}

Expand Messages | Collapse Messages