Show notifications on startup (Chrome)

background.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2013 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 90 matching lines @@
 {
   // Ignore fragment identifier
   var index = url.indexOf("#");
   if (index >= 0)
     url = url.substring(0, index);
 
   var result = defaultMatcher.matchesAny(url, type || "DOCUMENT", extractHostFromURL(parentUrl || url), false);
   return (result instanceof WhitelistFilter ? result : null);
 }
 
+var activeNotification = null;
+
 // Adds or removes page action icon according to options.
 function refreshIconAndContextMenu(tab)
 {
   // The tab could have been closed by the time this function is called
   if(!tab)
     return;
 
   var excluded = isWhitelisted(tab.url);
   var iconFilename = excluded ? "icons/abp-19-whitelisted.png" : "icons/abp-19.png";
-  chrome.pageAction.setIcon({tabId: tab.id, path: iconFilename});
+
+  if (activeNotification)
+    startIconAnimation(tab, iconFilename);
+  else
+    chrome.pageAction.setIcon({tabId: tab.id, path: iconFilename});
 
   // Only show icon for pages we can influence (http: and https:)
   if(/^https?:/.test(tab.url))
   {
     chrome.pageAction.setTitle({tabId: tab.id, title: "Adblock Plus"});
     if ("shouldShowIcon" in localStorage && localStorage["shouldShowIcon"] == "false")
       chrome.pageAction.hide(tab.id);
     else
       chrome.pageAction.show(tab.id);
 
     // Set context menu status according to whether current tab has whitelisted domain
     if (excluded)
       chrome.contextMenus.removeAll();
     else
       showContextMenu();
   }
-
-  if (activeNotification)
-    setNotificationPageAction(tab);

[Wladimir Palant, 2013/07/21 11:15:30]

This logic seems wrong to me - you are overriding refreshIconAndContextMenu()
logic here instead of extending it. Suggestion: you set the state in
showNotification() and refreshIconAndContextMenu() decides which icon to show
and whether to animate between two icons. If the state changes you can call
refreshIconAndContextMenu() for all tabs.

[Felix Dahlke, 2013/07/22 12:30:15]

Done.

 }
 
 /**
  * Old versions stored filter data in the localStorage object, this will import
  * it into FilterStorage properly.
  */
 function importOldData()
 {
   function addSubscription(url, title)
   {
@@ skipping 320 matching lines @@
             chrome.tabs.onUpdated.removeListener(listener);
             onLoad();
           }
         };
         chrome.tabs.onUpdated.addListener(listener);
       }
     });
   }
 }
 
-var activeNotification;

[Wladimir Palant, 2013/07/21 11:15:30]

Initialize with null so you don't work with undefined values in
refreshIconAndContextMenu?

[Felix Dahlke, 2013/07/22 12:30:15]

Done, didn't notice that we always tend to do that.

+var iconAnimationTimer = null;
+var animatedIconTab = null;
+
+function stopIconAnimation()
+{
+  if (!iconAnimationTimer)
+    return;
+
+  clearTimeout(iconAnimationTimer);
+  iconAnimationTimer = null;
+  animatedIconTab = null;
+}
 
 function loadImages(imageFiles, callback)
 {
   var images = {};
+  var imagesLoaded = 0;
   imageFiles.forEach(function(imageFile)
   {
     var image = new Image();
     image.src = imageFile;
     image.addEventListener("load", function()
     {
       images[imageFile] = image;
-      if (Object.keys(images).length === imageFiles.length)

[Wladimir Palant, 2013/07/21 11:15:30]

I really don't like relying on Object.keys() only having the keys you put into
it, use a proper counter?

[Felix Dahlke, 2013/07/22 12:30:15]

I had a hunch you'd say this :) Done.

+      if (++imagesLoaded === imageFiles.length)
         callback(images);
     });
   });
 }
 
-function startInformationNotificationAnimation()
-{
-  var abpIconFile = "icons/abp-19.png";

[Wladimir Palant, 2013/07/21 11:15:30]

What about "icons/abp-19-whitelisted.png"?

-  var notificationIconFile = "icons/notification-information.png";
-  var iconFiles = [abpIconFile, notificationIconFile];
+function startIconAnimation(tab, iconPath)
+{
+  stopIconAnimation();
+  animatedIconTab = tab;
+
+  var severitySuffix = activeNotification.severity === "critical"
+      ? "critical" : "information";
+  var notificationIconPath = "icons/notification-" + severitySuffix + ".png";
+  var iconFiles = [iconPath, notificationIconPath];
   loadImages(iconFiles, function(images)
   {
-    var abpIcon = images[abpIconFile];
-    var notificationIcon = images[notificationIconFile];
+    var icon = images[iconPath];
+    var notificationIcon = images[notificationIconPath];
+
     var canvas = document.createElement("canvas");
-    canvas.width = abpIcon.width;
-    canvas.height = abpIcon.height;
+    canvas.width = icon.width;
+    canvas.height = icon.height;
     var context = canvas.getContext("2d");
 
-    var animationStartTime = Date.now();
-    var animationInterval = 1000 / 60;
+    var currentFrame = 0;
+    var frameOpacities = [0, 0.2, 0.4, 0.6, 0.8,
+                          1, 1, 1, 1, 1,
+                          0.8, 0.6, 0.4, 0.2, 0];
+
     function animationStep()
     {
-      var timeElapsed = Date.now() - animationStartTime;
-      var duration = 3000;
-      var animationTime = timeElapsed % duration;
-      var delay = 1000;
-      var fadeInEndTime = duration / 2 - delay / 2;
-      var fadeOutStartTime = fadeInEndTime + delay;
-
-      var opacity;
-      if (animationTime < fadeInEndTime)
-        opacity = animationTime / fadeInEndTime;
-      else if (animationTime > fadeOutStartTime)
-        opacity = 1 - (animationTime - fadeOutStartTime)
-          / (duration - fadeOutStartTime);
-
+      var opacity = frameOpacities[currentFrame];
       context.clearRect(0, 0, canvas.width, canvas.height);
       context.globalAlpha = 1;
-      context.drawImage(abpIcon, 0, 0);
+      context.drawImage(icon, 0, 0);
       context.globalAlpha = opacity;
       context.drawImage(notificationIcon, 0, 0);
       var imageData = context.getImageData(0, 0, canvas.width, canvas.height);
-
-      chrome.tabs.getSelected(null, function(tab)
-      {
-        chrome.pageAction.setIcon({tabId: tab.id, imageData: imageData});
-        activeNotification.animationTimer =
-          setTimeout(animationStep, animationInterval);
-      });

[Wladimir Palant, 2013/07/21 11:15:30]

Frankly, drawing images 60 times per second is a huge overkill (e.g. think of
the CPU use). I suggest precomputing two images (abp-19.png with notification
icon and abp-19-whitelisted.png with notification icon). For the animation
switching between the regular icon and the version with notification every three
seconds should be sufficient.

[Felix Dahlke, 2013/07/21 12:19:53]

Well, drawing images 60 times per second is how animations normally work, and we
wouldn't be the only Chrome extension doing it. Note that this is more like
_at_most_ 60 times per second, so I thought we'd at worst get a stuttering
animation.

We could move to simple blinking, but I personally prefer the animation. In any
case, this is mostly an aesthetical issue, so we might want to delay that until
lthe next release, given that we're in a bit of a hurry. I'm sure we'll get
various ideas on how to improve this if I demonstrate it on Monday.

Regarding abp-19-whitelisted.png, I did try to get the path of the current icon
and use that, but that's not possible in Chrome so I didn't bother for now. We
can of course save the current icon path whenever we set it.

+      chrome.pageAction.setIcon({tabId: tab.id, imageData: imageData});
+
+      var interval;
+      currentFrame++;
+      if (currentFrame < frameOpacities.length)
+      {
+        var duration = 3000;
+        interval = duration / frameOpacities.length;
+      }
+      else
+      {
+        currentFrame = 0;
+        interval = 10000;
+      }
+      iconAnimationTimer = setTimeout(animationStep, interval);
     }
     animationStep();
   });
 }
 
-function showInformationNotification()
+function prepareNotificationIconAndPopup()
 {
   activeNotification.onClicked = function()
   {
-    clearTimeout(activeNotification.animationTimer);
-    chrome.tabs.getAllInWindow(null, function(tabs)
+    var tab = animatedIconTab;
+    stopIconAnimation();
+    activeNotification = null;
+    refreshIconAndContextMenu(tab);
+  };
+
+  chrome.windows.getLastFocused({populate: true}, function(window)
+  {
+    chrome.tabs.query({active: true, windowId: window.id}, function(tabs)
     {
       tabs.forEach(refreshIconAndContextMenu);
-      activeNotification = null;
     });
-  };
-
-  startInformationNotificationAnimation();
-}
-
-function showCriticalNotification()
-{
-  var notification = webkitNotifications.createHTMLNotification("notification.html");
-  notification.show();
-}
-
-function setNotificationPageAction(tab)
-{
-  if (activeNotification.severity !== "critical")
-    return;
-
-  var abpIconFile = "icons/abp-19.png";
-  var notificationIconFile = "icons/notification-critical.png";

[Wladimir Palant, 2013/07/21 11:15:30]

Again, what about abp-19-whitelisted.png?

Same comment here - we can precompile the images (e.g. in showNotification),
doing this all the time is unnecessary.

-  var iconFiles = [abpIconFile, notificationIconFile];
-  loadImages(iconFiles, function(images)
-  {
-    var abpIcon = images[abpIconFile];
-    var notificationIcon = images[notificationIconFile];
-    var canvas = document.createElement("canvas");
-    canvas.width = abpIcon.width;
-    canvas.height = abpIcon.height;
-    var context = canvas.getContext("2d");
-    context.clearRect(0, 0, canvas.width, canvas.height);
-    context.drawImage(abpIcon, 0, 0);
-    context.drawImage(notificationIcon, 0, 0);
-    var imageData = context.getImageData(0, 0, canvas.width, canvas.height);
-    chrome.pageAction.setIcon({tabId: tab.id, imageData: imageData});
   });
 }
 
 function showNotification(notification)
 {
   activeNotification = notification;
 
-  if (notification.severity === "critical")
-    showCriticalNotification();
+  if (activeNotification.severity === "critical")
+  {
+    var notification = webkitNotifications.createHTMLNotification("notification.html");
+    notification.show();
+    notification.addEventListener("close", prepareNotificationIconAndPopup);
+  }
   else
-    showInformationNotification();
-
-  chrome.tabs.getAllInWindow(null, function(tabs)
-  {
-    tabs.forEach(setNotificationPageAction);
-  });
-}
-
-function getLocalizedTexts(notification)
-{
-  return Notification.getLocalizedTexts(notification);
-}
-
-function getDocLinks(notification)
-{
-  if (!notification.links)
-    return [];
-
-  var docLinks = [];
-  notification.links.forEach(function(link)
-  {
-    docLinks.push(Utils.getDocLink(link));
-  });
-  return docLinks;
+    prepareNotificationIconAndPopup();
 }
 
 /**
  * This function is a hack - we only know the tabId and document URL for a
  * message but we need to know the frame ID. Try to find it in webRequest's
  * frame data.
  */
 function getFrameId(tabId, url)
 {
   if (tabId in frames)
@@ skipping 119 matching lines @@
 });
 
 // Update icon if a tab changes location
 chrome.tabs.onUpdated.addListener(function(tabId, changeInfo, tab)
 {
   chrome.tabs.sendRequest(tabId, {reqtype: "clickhide-deactivate"})
   if(changeInfo.status == "loading")
     refreshIconAndContextMenu(tab);
 });
 
+// Refresh icon when switching tabs or windows
+chrome.tabs.onActivated.addListener(function(activeInfo)
+{
+  refreshIconAndContextMenu(animatedIconTab);
+  chrome.tabs.get(activeInfo.tabId, refreshIconAndContextMenu);
+});
+chrome.windows.onFocusChanged.addListener(function(windowId)
+{
+  refreshIconAndContextMenu(animatedIconTab);
+  chrome.tabs.query({active: true, windowId: windowId}, function(tabs)
+  {
+    tabs.forEach(refreshIconAndContextMenu);
+  });
+});
+
 setTimeout(function()
 {
   var notificationToShow = Notification.getNextToShow();
   if (notificationToShow)
     showNotification(notificationToShow);
 }, 3 * 60 * 1000);