Issue 8351004: Add an extra argument to the DisplayBalloon method to support custom notification icons.

Issue 8351004: Add an extra argument to the DisplayBalloon method to support custom notification icons. (Closed)

Created:
9 years, 2 months ago by Leandro Gracia Gil

Modified:
9 years, 1 month ago

Reviewers:
Peter Kasting, Andrew T Wilson (Slow), Leandro Graciá Gil

CC:
chromium-reviews, Paweł Hajdan Jr.

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

Visibility:
Public.

Description

Add an extra argument to the DisplayBalloon method to support custom notification icons. This is the first of a set of patches introducing new functionality to the status icon API. This patch updates the API and adds missing OVERRIDE to the existing code. Per-platform functionality will be added in further patches. BUG=74970 TEST=build bots Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107329

Patch Set 1 #

Patch Set 2 : adding a missing comma. #

Patch Set 3 : adding 2 missing virtuals. #

Patch Set 4 : removing the observer overrides in the mock, as it seems to be cleaner. #

Patch Set 5 : fixing argument order as suggested. #

Total comments: 2

Created: 9 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+59 lines, -37 lines)			Patch
M	chrome/browser/background/background_mode_manager_win.cc	View	1 2 3 4	2 chunks	+2 lines, -0 lines	0 comments	Download
M	chrome/browser/status_icons/status_icon.h	View	1 2 3 4	2 chunks	+5 lines, -1 line	0 comments	Download
M	chrome/browser/status_icons/status_icon.cc	View		1 chunk	+1 line, -2 lines	0 comments	Download
M	chrome/browser/status_icons/status_icon_unittest.cc	View	1 2 3 4	2 chunks	+8 lines, -6 lines	0 comments	Download
M	chrome/browser/status_icons/status_tray_unittest.cc	View	1 2 3 4	2 chunks	+8 lines, -8 lines	0 comments	Download
M	chrome/browser/ui/cocoa/status_icons/status_icon_mac.h	View	1 2 3 4	3 chunks	+7 lines, -5 lines	0 comments	Download
M	chrome/browser/ui/cocoa/status_icons/status_icon_mac.mm	View	1 2 3 4	1 chunk	+2 lines, -1 line	0 comments	Download
M	chrome/browser/ui/cocoa/status_icons/status_tray_mac.h	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	chrome/browser/ui/gtk/status_icons/status_icon_gtk.h	View	1 2 3 4	2 chunks	+7 lines, -4 lines	0 comments	Download
M	chrome/browser/ui/gtk/status_icons/status_icon_gtk.cc	View	1 2 3 4	2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/gtk/status_icons/status_tray_gtk.h	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	chrome/browser/ui/views/status_icons/status_icon_win.h	View	1 2 3 4	2 chunks	+7 lines, -4 lines	0 comments	Download
M	chrome/browser/ui/views/status_icons/status_icon_win.cc	View	1 2 3 4	1 chunk	+4 lines, -1 line	2 comments	Download
M	chrome/browser/ui/views/status_icons/status_tray_win.h	View		2 chunks	+2 lines, -1 line	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

Leandro Graciá Gil

Andrew, please take a look to the discussed changes. Peter, can you take a look ...

9 years, 2 months ago (2011-10-24 14:51:27 UTC) #1

Peter Kasting

Rubber-stamp LGTM on ui/, with one nit: assuming you lay things out as: [icon] Title ...

9 years, 2 months ago (2011-10-24 21:45:40 UTC) #2

Andrew T Wilson (Slow)

LGTM, although I'd probably remove the TODO in status_icon_win.cc unless you actually think we should ...

9 years, 2 months ago (2011-10-26 00:38:18 UTC) #4

Leandro Graciá Gil

9 years, 2 months ago (2011-10-26 00:45:10 UTC) #5

http://codereview.chromium.org/8351004/diff/6001/chrome/browser/ui/views/stat...
File chrome/browser/ui/views/status_icons/status_icon_win.cc (right):

http://codereview.chromium.org/8351004/diff/6001/chrome/browser/ui/views/stat...
chrome/browser/ui/views/status_icons/status_icon_win.cc:87: //
TODO(leandrogracia): implement custom icons for notification balloons.
I have a working code for that and it's provided by
http://codereview.chromium.org/8379003/

On 2011/10/26 00:38:19, Andrew T Wilson wrote:
> Is there actually anything you can do on windows? I don't think the balloons
> will accept a custom icon.

Expand Messages | Collapse Messages