Initial implementation of Google Now Notifications integration

chrome/browser/resources/google_now/background.js

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+

[arv (Not doing code reviews), 2013/01/25 19:29:06]

'use strict';

[vadimt, 2013/01/25 21:48:28]

Done.

+/**
+ * @fileoverview The event page for Google Now for Chrome implementation.
+ * The Google Now event page gets Google Now cards from the server and shows
+ * them as Chrome notifications.
+ * The service performs periodic updating of Google Now cards.
+ * Each updating of the cards includes 3 steps:
+ * 1. Obtaining the location of the machine;
+ * 2. Making a server request based on that location;
+ * 3. Showing the received cards as notifications.
+ */
+
+// TODO(vadimt): Use background permission to show notifications even when all
+// browser windows are closed.
+// TODO(vadimt): Remove the C++ implementation.
+// TODO(vadimt): Decide what to do in incognito mode.
+// TODO(vadimt): Gather UMAs.
+// TODO(vadimt): Honor the flag the enables Google Now integration.
+// TODO(vadimt): Figure out the final values of the constants.
+// TODO(vadimt): Report errors to the user.
+
+// TODO(vadimt): Figure out the server name. Use it in the manifest and for
+// NOTIFICATION_CARDS_URL. Meanwhile, to use the feature, you need to manually
+// edit NOTIFICATION_CARDS_URL before building Chrome.
+/**
+ * URL to retrieve notification cards.
+ */
+var NOTIFICATION_CARDS_URL = '';
+
+/**
+ * Standard response code for successful HTTP requests. This is the only success
+ * code the server will send.
+ */
+var HTTP_OK = 200;
+
+/**
+ * Period for polling for Google Now Notifications cards to use when the period
+ * from the server is not available.
+ */
+var DEFAULT_POLLING_PERIOD_SECONDS = 300;  // 5 minutes
+
+/**
+ * Parse JSON response of the notifications server, show notifications and
+ * schedule next update.
+ * @param {string} response Server response.
+ */
+function ParseAndShowNotificationCards(response) {

[Matt Perry, 2013/01/24 18:34:38]

JavaScript style is to use camelCase naming for functions:
http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml?showone...

[vadimt, 2013/01/25 21:48:28]

Done.

+  try {
+    var parsedResponse = JSON.parse(response);
+  } catch (error) {
+    return;
+  }
+
+  var cards = parsedResponse.cards;
+
+  if (!(cards instanceof Array))
+    return;
+
+  if (typeof parsedResponse.expiration_timestamp_seconds != 'number')
+    return;
+
+  for (var i = 0; i != cards.length; ++i) {

[arv (Not doing code reviews), 2013/01/25 19:29:06]

cards.forEach?

[vadimt, 2013/01/25 21:48:28]

With forEach, the error processing code for notification.show() will be less
clear.
Here, I simply call 'return' from try-catch.
With forEach, the code will be more tricky (bContinue flag etc.), or I'll have
to place try-catch around whole loop, making code less readable (not clear who
throws an exception that we are trying to catch).

+    try {
+      chrome.experimental.notification.show(cards[i], function(showInfo) {});
+    } catch (error) {
+      return;

[arv (Not doing code reviews), 2013/01/25 19:29:06]

why? Please add comments every time you silence an exception.

[vadimt, 2013/01/25 21:48:28]

Done.

+    }
+  }
+
+  ScheduleNextUpdate(parsedResponse.expiration_timestamp_seconds);
+}
+
+/**
+ * Request notification cards from the server.
+ * @param {string} requestParameters Query string for the request.
+ */
+function RequestNotificationCards(requestParameters) {
+  // TODO(vadimt): Figure out how to send user's identity to the server.
+  var request = new XMLHttpRequest();

[arv (Not doing code reviews), 2013/01/25 19:29:06]

You might want to set the responseType to text so that no Document is created.

[vadimt, 2013/01/25 21:48:28]

Done.

+  request.onreadystatechange = function(event) {

[arv (Not doing code reviews), 2013/01/25 19:29:06]

you can do onload instead

[vadimt, 2013/01/25 21:48:28]

Done.

+    if (request.readyState == request.DONE && request.status == HTTP_OK)
+      ParseAndShowNotificationCards(request.response);
+  }
+  request.open('GET', NOTIFICATION_CARDS_URL + requestParameters, true);
+  request.send();
+}
+
+/**
+ * Request notification cards from the server when we have geolocation.
+ * @param {Geoposition} position Location of this computer.
+ */
+function RequestNotificationCardsWithLocation(position) {
+  // TODO(vadimt): Should we use 'q' as the parameter name?
+  var requestParameters =
+      '?q=' + position.coords.latitude +
+      ',' + position.coords.longitude +
+      ',' + position.coords.accuracy;
+
+  RequestNotificationCards(requestParameters);
+}
+
+/**
+ * Request notification cards from the server when we don't have geolocation.
+ * @param {PositionError} positionError Position error.
+ */
+function RequestNotificationCardsWithoutLocation(positionError) {
+  RequestNotificationCards('');
+}
+
+/**
+ * Obtain new location; request and show notification cards based on this
+ * location.
+ */
+function UpdateNotificationsCards() {
+  // Immediately schedule the update after the default period. If we
+  // successfully retrieve, parse and show the notifications cards, we'll
+  // schedule next update based on the expiration timestamp received from the
+  // server. At that point scheduled time will be overwritten by the new one
+  // based on the expiration timestamp.
+  // TODO(vadimt): Implement exponential backoff with randomized jitter.
+  ScheduleNextUpdate(DEFAULT_POLLING_PERIOD_SECONDS);
+
+  // TODO(vadimt): Use chrome.* geolocation API once it's ready.
+  navigator.geolocation.getCurrentPosition(
+      RequestNotificationCardsWithLocation,
+      RequestNotificationCardsWithoutLocation);
+}
+
+/**
+ * Schedule next update for notification cards.
+ * @param {int} delaySeconds Length of time in seconds after which the alarm
+ *     event should fire.
+ */
+function ScheduleNextUpdate(delaySeconds) {
+  chrome.alarms.create({ when: Date.now() + delaySeconds * 1000 });

[Matt Perry, 2013/01/24 18:34:38]

nit: could also use: chrome.alarms.create({delayInMinutes: delaySeconds / 60});

(or just specify the delay in minutes everywhere.

[vadimt, 2013/01/25 21:48:28]

Yes, but chrome.alarms sometimes rounds delay to a minute, and I used to see it
rounding to 5 mins IIRC. By specifying absolute time, I avoid the issue.

[Matt Perry, 2013/01/25 22:05:34]

That's not true. delayInMinutes is a double, so you can specify fractional
values. There is still a minimum delay of 1 minute, but that applies when using
"when" as well.

[vadimt, 2013/01/25 23:01:19]

You are right!
Changed to delayInMinutes, but still want to have seconds across the code, since
Google Now server sends seconds.

+}
+
+/**
+ * Initialize the event page on install or on browser startup.
+ */
+function Initialize() {
+  UpdateNotificationsCards();
+}
+
+chrome.runtime.onInstalled.addListener(function(details) {
+  if (details.reason != 'chrome_update')
+    Initialize();
+});
+
+chrome.runtime.onStartup.addListener(function() {
+  Initialize();

[Matt Perry, 2013/01/24 18:34:38]

Do you need to update the cards immediately on startup? Alarms are persisted
across browser restart. That means if the alarm has expired since you last
closed chrome, it will dispatch the event when you start chrome. If it hasn't
expired yet, the alarm will fire when it expires.

[vadimt, 2013/01/25 21:48:28]

Correct. However, we specifically want to update the cards on the browser's
startup. This is a product decision.

+});
+
+chrome.alarms.onAlarm.addListener(function(alarm) {
+  UpdateNotificationsCards();
+});