[GCM] Adding the OnSendAcknowledgement event

[GCM] Adding the OnSendAcknowledgement event

Adding the OnSendAcknowledgement event to enable Chrome components to
get updates on GCM server receiving the messages.

BUG=374969
TBR=kalman@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287831

[fgorski, 2014-08-05 23:33:08]

PTAL

zea: everything except c/b/extension
kalman: c/b/extensions (I've put you as a TBR, as there is no logic just a blunt
handler.)

[Nicolas Zea, 2014-08-05 23:39:48]

LGTM with some comments

https://codereview.chromium.org/442123003/diff/1/components/gcm_driver/defaul...
File components/gcm_driver/default_gcm_app_handler.cc (right):

https://codereview.chromium.org/442123003/diff/1/components/gcm_driver/defaul...
components/gcm_driver/default_gcm_app_handler.cc:42: LOG(ERROR) << "No app
handler is found to route send acknoledgement for "
Will this get hit if we send a message after uninstalling the app? Maybe just
DVLOG if it's expected?

https://codereview.chromium.org/442123003/diff/1/components/gcm_driver/gcm_cl...
File components/gcm_driver/gcm_client_impl_unittest.cc (left):

https://codereview.chromium.org/442123003/diff/1/components/gcm_driver/gcm_cl...
components/gcm_driver/gcm_client_impl_unittest.cc:681:
mcs_proto::DataMessageStanza stanza;
Why was this removed?

[fgorski, 2014-08-06 16:22:27]

Applied and explained.

https://codereview.chromium.org/442123003/diff/1/components/gcm_driver/defaul...
File components/gcm_driver/default_gcm_app_handler.cc (right):

https://codereview.chromium.org/442123003/diff/1/components/gcm_driver/defaul...
components/gcm_driver/default_gcm_app_handler.cc:42: LOG(ERROR) << "No app
handler is found to route send acknoledgement for "
On 2014/08/05 23:39:48, Nicolas Zea wrote:
> Will this get hit if we send a message after uninstalling the app? Maybe just
> DVLOG if it's expected?

Done. That is a valid scenario, and I updated the whole file to use DVLOGs
instead of LOG(ERROR)

https://codereview.chromium.org/442123003/diff/1/components/gcm_driver/gcm_cl...
File components/gcm_driver/gcm_client_impl_unittest.cc (left):

https://codereview.chromium.org/442123003/diff/1/components/gcm_driver/gcm_cl...
components/gcm_driver/gcm_client_impl_unittest.cc:681:
mcs_proto::DataMessageStanza stanza;
On 2014/08/05 23:39:48, Nicolas Zea wrote:
> Why was this removed?

Because it is not needed.
It seems that when the test was originally defined, this was extra couple of
lines that shouldn't have make it. If you look below the

message.time_to_live = 500;

is pretty much the same logic. Variable "stanza" is never used in this test
other than to set TTL.

I was looking at this test to make the one which acknowledges, and accidentally
found these 3 lines.

[fgorski, 2014-08-06 16:23:07]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2014-08-06 16:24:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/fgorski@chromium.org/442123003/40001

[commit-bot: I haz the power, 2014-08-06 19:42:50]

Change committed as 287831