Receiving and sending dismissal parameters

Receiving and sending dismissal parameters.

BUG=164227
TEST=Please speak to the Server Person regarding types of dismissals and specific testing for them.

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

[rgustafson, 2013-06-27 00:23:27]

lgtm

[rgustafson, 2013-06-27 00:23:57]

whoops. my bad. wrong review. I take that back. ;)

[rgustafson, 2013-06-27 02:32:39]

https://codereview.chromium.org/17894010/diff/2001/chrome/browser/resources/g...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/17894010/diff/2001/chrome/browser/resources/g...
chrome/browser/resources/google_now/background.js:478: var requestParameters =
I wanted to just send a json object in the post body with the format from my
email. It makes the API definition much simpler. Unfortunately, the relative
times don't make it much simpler here.

But you can just stringify the whole thing then.

Then the content type would be "application/json". I want to make this behave
the same way for notification requests too (goodbye q), but thats a different
change.

[vadimt, 2013-06-27 19:52:56]

https://codereview.chromium.org/17894010/diff/2001/chrome/browser/resources/g...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/17894010/diff/2001/chrome/browser/resources/g...
chrome/browser/resources/google_now/background.js:478: var requestParameters =
On 2013/06/27 02:32:40, rgustafson wrote:
> I wanted to just send a json object in the post body with the format from my
> email. It makes the API definition much simpler. Unfortunately, the relative
> times don't make it much simpler here.
> 
> But you can just stringify the whole thing then.
> 
> Then the content type would be "application/json". I want to make this behave
> the same way for notification requests too (goodbye q), but thats a different
> change.

Done.

[skare_, 2013-07-10 18:47:52]

lgtm

[rgustafson, 2013-07-10 22:14:40]

lgtm

lgtm for real this time

[vadimt, 2013-07-10 22:15:57]

arv@, please provide OWNER's approval.

[arv (Not doing code reviews), 2013-07-10 22:19:57]

LGTM

https://codereview.chromium.org/17894010/diff/12001/chrome/browser/resources/...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/17894010/diff/12001/chrome/browser/resources/...
chrome/browser/resources/google_now/background.js:543: function(success) {
strange indentation

[vadimt, 2013-07-10 22:47:18]

https://codereview.chromium.org/17894010/diff/12001/chrome/browser/resources/...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/17894010/diff/12001/chrome/browser/resources/...
chrome/browser/resources/google_now/background.js:543: function(success) {
On 2013/07/10 22:19:57, arv wrote:
> strange indentation

Done.

[commit-bot: I haz the power, 2013-07-10 22:47:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vadimt@chromium.org/17894010/23001

[commit-bot: I haz the power, 2013-07-20 01:09:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vadimt@chromium.org/17894010/23001

[commit-bot: I haz the power, 2013-07-20 01:09:50]

Failed to apply patch for chrome/browser/resources/google_now/background.js:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/resources/google_now/background.js
  Hunk #1 succeeded at 281 (offset 7 lines).
  Hunk #2 succeeded at 413 (offset 7 lines).
  Hunk #3 FAILED at 468.
  Hunk #4 succeeded at 511 (offset 20 lines).
  Hunk #5 FAILED at 540.
  Hunk #6 succeeded at 673 (offset 20 lines).
  2 out of 6 hunks FAILED -- saving rejects to file
chrome/browser/resources/google_now/background.js.rej

Patch:       chrome/browser/resources/google_now/background.js
Index: chrome/browser/resources/google_now/background.js
diff --git a/chrome/browser/resources/google_now/background.js
b/chrome/browser/resources/google_now/background.js
index
e651a18c5676448240c59511113df7ba9a95070e..f2e07dbfdbc6438c3e897512b72931256e9f68f7
100644
--- a/chrome/browser/resources/google_now/background.js
+++ b/chrome/browser/resources/google_now/background.js
@@ -274,7 +274,8 @@ function showNotification(card, notificationsData,
opt_previousVersion) {
 
   notificationsData[card.notificationId] = {
     actionUrls: card.actionUrls,
-    version: card.version
+    version: card.version,
+    dismissalParameters: card.dismissal
   };
 }
 
@@ -405,7 +406,8 @@ function requestNotificationCards(position, callback) {
       ',' + position.coords.longitude +
       ',' + position.coords.accuracy;
 
-  var request = buildServerRequest('notifications');
+  var request = buildServerRequest('notifications',
+                                   'application/x-www-form-urlencoded');
 
   request.onloadend = function(event) {
     console.log('requestNotificationCards-onloadend ' + request.status);
@@ -466,18 +468,16 @@ function updateNotificationsCards(position) {
  * @param {string} notificationId Unique identifier of the card.
  * @param {number} dismissalTimeMs Time of the user's dismissal of the card in
  *     milliseconds since epoch.
+ * @param {Object} dismissalParameters Dismissal parameters.
  * @param {function(boolean)} callbackBoolean Completion callback with
'success'
  *     parameter.
  */
 function requestCardDismissal(
-    notificationId, dismissalTimeMs, callbackBoolean) {
+    notificationId, dismissalTimeMs, dismissalParameters, callbackBoolean) {
   console.log('requestDismissingCard ' + notificationId + ' from ' +
       NOTIFICATION_CARDS_URL);
   recordEvent(DiagnosticEvent.DISMISS_REQUEST_TOTAL);
-  // Send a dismiss request to the server.
-  var requestParameters = 'id=' + notificationId +
-                          '&dismissalAge=' + (Date.now() - dismissalTimeMs);
-  var request = buildServerRequest('dismiss');
+  var request = buildServerRequest('dismiss', 'application/json');
   request.onloadend = function(event) {
     console.log('requestDismissingCard-onloadend ' + request.status);
     if (request.status == HTTP_OK)
@@ -489,7 +489,13 @@ function requestCardDismissal(
   setAuthorization(request, function(success) {
     if (success) {
       tasks.debugSetStepName('requestCardDismissal-send-request');
-      request.send(requestParameters);
+
+      var dismissalObject = {
+        id: notificationId,
+        age: Date.now() - dismissalTimeMs,
+        dismissal: dismissalParameters
+      };
+      request.send(JSON.stringify(dismissalObject));
     } else {
       callbackBoolean(false);
     }
@@ -532,16 +538,19 @@ function processPendingDismissals(callbackBoolean) {
       // recursively with the rest.
       var dismissal = items.pendingDismissals[0];
       requestCardDismissal(
-          dismissal.notificationId, dismissal.time, function(success) {
-        if (success) {
-          dismissalsChanged = true;
-          items.pendingDismissals.splice(0, 1);
-          items.recentDismissals[dismissal.notificationId] = Date.now();
-          doProcessDismissals();
-        } else {
-          onFinish(false);
-        }
-      });
+          dismissal.notificationId,
+          dismissal.time,
+          dismissal.parameters,
+          function(success) {
+            if (success) {
+              dismissalsChanged = true;
+              items.pendingDismissals.splice(0, 1);
+              items.recentDismissals[dismissal.notificationId] = Date.now();
+              doProcessDismissals();
+            } else {
+              onFinish(false);
+            }
+          });
     }
 
     doProcessDismissals();
@@ -645,13 +654,17 @@ function onNotificationClosed(notificationId, byUser) {
         notificationId,
         function() {});
 
-    tasks.debugSetStepName('onNotificationClosed-get-pendingDismissals');
-    storage.get('pendingDismissals', function(items) {
+    tasks.debugSetStepName('onNotificationClosed-storage-get');
+    storage.get(['pendingDismissals', 'notificationsData'], function(items) {
       items.pendingDismissals = items.pendingDismissals || [];
+      items.notificationsData = items.notificationsData || {};
+
+      var notificationData = items.notificationsData[notificationId];
 
       var dismissal = {
         notificationId: notificationId,
-        time: Date.now()
+        time: Date.now(),
+        parameters: notificationData && notificationData.dismissalParameters
       };
       items.pendingDismissals.push(dismissal);
       storage.set({pendingDismissals: items.pendingDismissals});

[commit-bot: I haz the power, 2013-07-20 01:56:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vadimt@chromium.org/17894010/42001

[commit-bot: I haz the power, 2013-07-20 06:06:40]

Change committed as 212758