Issue 6392014: [Sync] Relax checks for received XMPP messages

Issue 6392014: [Sync] Relax checks for received XMPP messages (Closed)

Created:
9 years, 10 months ago by akalin

Modified:
9 years, 7 months ago

Reviewers:
sanjeevr

CC:
chromium-reviews, Raghu Simha, ncarter (slow), tim (not reviewing), idana

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

Visibility:
Public.

Description

[Sync] Relax checks for received XMPP messages Basically they caused more bugs than they fixed (see discussion in bug). BUG=71285 TEST= Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=73180

Patch Set 1 #

Total comments: 4

Patch Set 2 : Addressed sanjeev's comments #

Created: 9 years, 10 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+10 lines, -16 lines)			Patch
	M	chrome/browser/sync/notifier/cache_invalidation_packet_handler.cc	View	1	1 chunk	+3 lines, -9 lines	0 comments	Download
	M	jingle/notifier/listener/listen_task.cc	View	1	3 chunks	+7 lines, -7 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

sanjeevr

LGTM http://codereview.chromium.org/6392014/diff/1/chrome/browser/sync/notifier/cache_invalidation_packet_handler.cc File chrome/browser/sync/notifier/cache_invalidation_packet_handler.cc (right): http://codereview.chromium.org/6392014/diff/1/chrome/browser/sync/notifier/cache_invalidation_packet_handler.cc#newcode87 chrome/browser/sync/notifier/cache_invalidation_packet_handler.cc:87: return MatchRequestIq(stanza, buzz::STR_SET, kQnData); Nit: Might want to ...

9 years, 10 months ago (2011-01-30 22:55:06 UTC) #2

akalin

9 years, 10 months ago (2011-01-31 09:54:17 UTC) #3

Checking in once trybots pass.

http://codereview.chromium.org/6392014/diff/1/chrome/browser/sync/notifier/ca...
File chrome/browser/sync/notifier/cache_invalidation_packet_handler.cc (right):

http://codereview.chromium.org/6392014/diff/1/chrome/browser/sync/notifier/ca...
chrome/browser/sync/notifier/cache_invalidation_packet_handler.cc:87: return
MatchRequestIq(stanza, buzz::STR_SET, kQnData);
On 2011/01/30 22:55:07, sanjeevr wrote:
> Nit: Might want to add some comments mentioning the reasoning behind the
limited
> validation.

Done.

http://codereview.chromium.org/6392014/diff/1/jingle/notifier/listener/listen...
File jingle/notifier/listener/listen_task.cc (right):

http://codereview.chromium.org/6392014/diff/1/jingle/notifier/listener/listen...
jingle/notifier/listener/listen_task.cc:130: return MatchRequestIq(stanza,
buzz::STR_SET, kQnNotifierGetAll);
On 2011/01/30 22:55:07, sanjeevr wrote:
> Nit: Leave the original comment about the format of the notification.

Done.

Expand Messages | Collapse Messages