Issue 6312178: Highlight background pages in task manager if user clicks View Background Pages.

Issue 6312178: Highlight background pages in task manager if user clicks View Background Pages. (Closed)

Created:
9 years, 10 months ago by Andrew T Wilson (Slow)

Modified:
9 years, 7 months ago

Reviewers:
John Gregg, pink (ping after 24hrs)

CC:
chromium-reviews, pam+watch_chromium.org, Paweł Hajdan Jr.

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

Visibility:
Public.

Description

Highlight background pages in task manager if user clicks View Background Pages. Added "highlight_background_resources" param to BrowserWindow::ShowTaskManager() and added support in Cocoa for drawing highlights. BUG=71490 TEST=Click View Background Pages, see background pages are highlighted in yellow. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=73993

Patch Set 1 #

Patch Set 2 : Fixed GTK compilation errors #

Total comments: 3

Patch Set 3 : Review feedback #

Created: 9 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+115 lines, -25 lines)			Patch
M	chrome/browser/app_controller_mac.mm	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/background_mode_manager.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/browser.h	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/ui/browser.cc	View	1	2 chunks	+7 lines, -4 lines	0 comments	Download
M	chrome/browser/ui/browser_window.h	View	1	1 chunk	+3 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/cocoa/browser_window_cocoa.h	View	1	1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/browser/ui/cocoa/browser_window_cocoa.mm	View	1	1 chunk	+5 lines, -1 line	0 comments	Download
M	chrome/browser/ui/cocoa/task_manager_mac.h	View		6 chunks	+16 lines, -4 lines	0 comments	Download
M	chrome/browser/ui/cocoa/task_manager_mac.mm	View	1 2	6 chunks	+62 lines, -10 lines	0 comments	Download
M	chrome/browser/ui/cocoa/task_manager_mac_unittest.mm	View	1	3 chunks	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/ui/gtk/browser_window_gtk.h	View	1	1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/browser/ui/gtk/browser_window_gtk.cc	View	1	1 chunk	+6 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/views/frame/browser_view.h	View	1	1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/browser/ui/views/frame/browser_view.cc	View	1	1 chunk	+6 lines, -0 lines	0 comments	Download
M	chrome/test/test_browser_window.h	View	1	1 chunk	+1 line, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Andrew T Wilson (Slow)

If either of you have cycles to look at this, I'd appreciate it. I'm trying ...

9 years, 10 months ago (2011-02-06 06:13:43 UTC) #1

pink (ping after 24hrs)

LGTM with small nits. http://codereview.chromium.org/6312178/diff/2001/chrome/browser/ui/cocoa/task_manager_mac.mm File chrome/browser/ui/cocoa/task_manager_mac.mm (right): http://codereview.chromium.org/6312178/diff/2001/chrome/browser/ui/cocoa/task_manager_mac.mm#newcode384 chrome/browser/ui/cocoa/task_manager_mac.mm:384: - (void)tableView:(NSTableView*)tableView comment for method? ...

9 years, 10 months ago (2011-02-07 15:59:08 UTC) #2

Andrew T Wilson (Slow)

9 years, 10 months ago (2011-02-07 16:53:46 UTC) #3

On 2011/02/07 15:59:08, pink wrote:
> LGTM with small nits.
> 
>
http://codereview.chromium.org/6312178/diff/2001/chrome/browser/ui/cocoa/task...
> File chrome/browser/ui/cocoa/task_manager_mac.mm (right):
> 
>
http://codereview.chromium.org/6312178/diff/2001/chrome/browser/ui/cocoa/task...
> chrome/browser/ui/cocoa/task_manager_mac.mm:384: -
> (void)tableView:(NSTableView*)tableView
> comment for method?

Done.

> 
>
http://codereview.chromium.org/6312178/diff/2001/chrome/browser/ui/cocoa/task...
> chrome/browser/ui/cocoa/task_manager_mac.mm:385: willDisplayCell:(id)cell
> minimum indent 4 spaces. That wins over lining up colons.
> 

Done.

>
http://codereview.chromium.org/6312178/diff/2001/chrome/browser/ui/cocoa/task...
> chrome/browser/ui/cocoa/task_manager_mac.mm:597: if (instance_ == this)
> when would this be not true? comment perhaps?
I thought there might be a delay between closing the TaskManager and having the
callback invoked, but there's no delay. I just removed this if statement.

Expand Messages | Collapse Messages