Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(728)

Issue 793883003: [GCM] Fix gcm network change handling before connect (Closed)

Created:
6 years ago by Nicolas Zea
Modified:
6 years ago
Reviewers:
jianli
CC:
chromium-reviews, zea+watch_chromium.org, fgorski
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

[GCM] Fix gcm network change handling before connect We were listening to network events and trying to reconnect before even being told to do the first connection. Until Connect() has been called, SignalConnectionReset should have no effect. BUG=440455 Committed: https://crrev.com/e0b18536e2f4f879545c688821ba378bc64ccd76 Cr-Commit-Position: refs/heads/master@{#307965}

Patch Set 1 #

Patch Set 2 : self review #

Patch Set 3 : Self check #

Patch Set 4 : Fix leak #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -15 lines) Patch
M google_apis/gcm/engine/connection_factory_impl.h View 1 2 chunks +6 lines, -0 lines 0 comments Download
M google_apis/gcm/engine/connection_factory_impl.cc View 3 chunks +17 lines, -7 lines 0 comments Download
M google_apis/gcm/engine/connection_factory_impl_unittest.cc View 1 2 3 6 chunks +21 lines, -7 lines 0 comments Download
M google_apis/gcm/engine/mcs_client.cc View 1 2 3 2 chunks +1 line, -1 line 0 comments Download

Messages

Total messages: 11 (4 generated)
Nicolas Zea
+Jian, PTAL
6 years ago (2014-12-11 00:08:51 UTC) #2
jianli
lgtm
6 years ago (2014-12-11 00:27:57 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/793883003/40001
6 years ago (2014-12-11 00:31:40 UTC) #5
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_asan_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_asan_rel/builds/17035)
6 years ago (2014-12-11 03:17:28 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/793883003/60001
6 years ago (2014-12-11 19:48:34 UTC) #9
commit-bot: I haz the power
Committed patchset #4 (id:60001)
6 years ago (2014-12-11 20:44:52 UTC) #10
commit-bot: I haz the power
6 years ago (2014-12-11 20:46:28 UTC) #11
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/e0b18536e2f4f879545c688821ba378bc64ccd76
Cr-Commit-Position: refs/heads/master@{#307965}

Powered by Google App Engine
This is Rietveld 408576698