Issue 2817037: Refactored ChromeInvalidationClient to have its own listener type.

Issue 2817037: Refactored ChromeInvalidationClient to have its own listener type. (Closed)

Created:
10 years, 5 months ago by akalin

Modified:
9 years, 7 months ago

Reviewers:
ncarter (slow)

CC:
chromium-reviews, ncarter (slow), ben+cc_chromium.org, Raghu Simha, idana, pam+watch_chromium.org, tim (not reviewing)

Visibility:
Public.

Description

Refactored ChromeInvalidationClient to have its own listener type. BUG=34647 TEST=manual Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51178

Patch Set 2 : synced to head #

Total comments: 6

Patch Set 3 : Addressed nick's comments #

Created: 10 years, 5 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+149 lines, -213 lines)			Patch
M	chrome/browser/sync/notifier/chrome_invalidation_client.h	View	1 2	2 chunks	+33 lines, -10 lines	0 comments	Download
M	chrome/browser/sync/notifier/chrome_invalidation_client.cc	View	1	2 chunks	+96 lines, -12 lines	0 comments	Download
M	chrome/browser/sync/notifier/server_notifier_thread.h	View	1 2	3 chunks	+6 lines, -22 lines	0 comments	Download
M	chrome/browser/sync/notifier/server_notifier_thread.cc	View	1	5 chunks	+5 lines, -75 lines	0 comments	Download
M	chrome/browser/sync/tools/sync_listen_notifications.cc	View		6 chunks	+9 lines, -93 lines	0 comments	Download
M	chrome/browser/sync/tools/sync_tools.gyp	View		1 chunk	+0 lines, -1 line	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

akalin

On 2010/06/29 03:28:08, akalin wrote: > +nick for review Ping!

10 years, 5 months ago (2010-06-29 18:21:08 UTC) #2

ncarter (slow)

LGTM, nits. http://codereview.chromium.org/2817037/diff/7001/8002 File chrome/browser/sync/notifier/chrome_invalidation_client.h (right): http://codereview.chromium.org/2817037/diff/7001/8002#newcode39 chrome/browser/sync/notifier/chrome_invalidation_client.h:39: virtual void OnInvalidateAll() = 0; DISALLOW_COPY_AND_ASSIGN() http://codereview.chromium.org/2817037/diff/7001/8002#newcode45 ...

10 years, 5 months ago (2010-06-29 19:53:35 UTC) #3

akalin

10 years, 5 months ago (2010-06-29 20:55:20 UTC) #4

Submitting as soon as trybots pass.

http://codereview.chromium.org/2817037/diff/7001/8002
File chrome/browser/sync/notifier/chrome_invalidation_client.h (right):

http://codereview.chromium.org/2817037/diff/7001/8002#newcode39
chrome/browser/sync/notifier/chrome_invalidation_client.h:39: virtual void
OnInvalidateAll() = 0;
On 2010/06/29 19:53:36, ncarter wrote:
> DISALLOW_COPY_AND_ASSIGN()

I don't think DISALLOW_COPY_AND_ASSIGN() is meant for interface classes.

http://codereview.chromium.org/2817037/diff/7001/8002#newcode45
chrome/browser/sync/notifier/chrome_invalidation_client.h:45:
~ChromeInvalidationClient();
On 2010/06/29 19:53:36, ncarter wrote:
> virtual

Done.

http://codereview.chromium.org/2817037/diff/7001/8004
File chrome/browser/sync/notifier/server_notifier_thread.h (right):

http://codereview.chromium.org/2817037/diff/7001/8004#newcode48
chrome/browser/sync/notifier/server_notifier_thread.h:48: //
ChromeInvalidationClient::InvalidationListener implementation.
On 2010/06/29 19:53:36, ncarter wrote:
> You mean ::Listener, not ::InvalidationListener

Done.

Expand Messages | Collapse Messages