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 103 matching lines @@
 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";
 
   if (activeNotification)
-    setNotificationPageAction(tab, iconFilename);

[Wladimir Palant, 2013/07/22 14:18:36]

setNotificationPageAction() won't do anything for non-critical notifications -
does this mean that we don't set any icon then until the animation catches up?
Should be if (activeNotification && activeNotification.severify == "critical) I
think.

[Felix Dahlke, 2013/07/22 15:10:22]

Yes, it was like that. In patch set 4, all notification icons are animated, so
that's the case for both information and critical now. But since the animation
is updated every 200ms, that seems fine.

+    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();
   }
-}
-
-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 (++imagesLoaded === imageFiles.length)
-        callback(images);
-    });
-  });
-}
-
-var iconAnimationTimer = null;
-
-function stopIconAnimation()
-{
-  if (!iconAnimationTimer)
-    return;
-
-  clearTimeout(iconAnimationTimer);
-  iconAnimationTimer = null;
-  chrome.tabs.getAllInWindow(null, function(tabs)
-  {
-    tabs.forEach(refreshIconAndContextMenu);
-    activeNotification = null;

[Wladimir Palant, 2013/07/22 14:18:36]

Shouldn't activeNotification be reset before calling
refreshIconAndContextMenu()?

Then again, resetting it here seems to be the wrong thing to do anyway - neither
required nor really expected. Mind you, this is an async action so it will reset
activeNotification at some random point in time.

[Felix Dahlke, 2013/07/22 15:10:22]

Yes, I fixed that in patch set 4.

-  });
-}
-
-function setNotificationPageAction(tab, iconFileName)
-{
-  if (activeNotification.severity !== "critical")
-    return;
-
-  stopIconAnimation();
-
-  var notificationIconFile = "icons/notification-critical.png";
-  var iconFiles = [iconFileName, notificationIconFile];
-  loadImages(iconFiles, function(images)
-  {
-    var abpIcon = images[iconFileName];
-    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);

[Wladimir Palant, 2013/07/22 14:18:36]

The code to combine two images with some opacity and get image data back could
be moved into a separate function - to be reused in
startInformationNotificationAnimation().

[Felix Dahlke, 2013/07/22 15:10:22]

Not necessary anymore, as of patch set 4.

-    chrome.pageAction.setIcon({tabId: tab.id, imageData: imageData});
-  });
 }
 
 /**
  * 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);
       }
     });
   }
 }
 
-function startInformationNotificationAnimation()
+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 (++imagesLoaded === imageFiles.length)
+        callback(images);
+    });
+  });
+}
+
+function startIconAnimation(tab, iconPath)
 {
   stopIconAnimation();
-
-  var abpIconFile = "icons/abp-19.png";

[Wladimir Palant, 2013/07/22 14:18:36]

This still hardcodes the icon and fails to consider that there can be two. This
is why I asked for the animation to be started from refreshIconAndContextMenu()
(and restarted for each tab switch) - you are building up a second control flow
for the same object here, with duplicated logic.

[Felix Dahlke, 2013/07/22 15:10:22]

Has been fixed in patch set 4. But yeah, second control flow. I considered
starting an animation for each tab, but that means we have one timer for every
tab, seems wasteful.

[Wladimir Palant, 2013/07/23 07:25:04]

No, we can still work with a single global timer - refreshIconAndContextMenu()
would simply clear it and create a new one for the current tab.

[Felix Dahlke, 2013/07/23 10:38:08]

Done. I do prefer this, just wasn't able to come up with it. Well, at least not
in a hurry :)

-  var notificationIconFile = "icons/notification-information.png";
-  var iconFiles = [abpIconFile, notificationIconFile];
+  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 / 10;
+    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);

[Wladimir Palant, 2013/07/22 14:18:36]

Pretty complicated and IMHO not really necessary calculations:

var animationInterval = 200;
var currentStep = 0;
var opacityValues = [0.0, 0.2, 0.4, 0.6, 0.8,
                     1.0, 1.0, 1.0, 1.0, 1.0,
                     0.8, 0.6, 0.4, 0.2, 0.0];
function animationStep()
{
  var opacity = opacityValues[++currentStep % opacityValues.length];
  ...

  if (currentStep % (opacityValues.length * 3) == 0)
    setTimeout(animationStep, 60000);
  else
    setTimeout(animationStep, animationInteval);
}

Note that Date.now() isn't exactly precise (e.g. its value is known to be
cached) and you using async actions in between distorts things even further.
IMHO you shouldn't try to bind this to some specific time.

[Felix Dahlke, 2013/07/22 15:10:22]

It's not really possible to get smooth animations without adjusting for the time
between steps, but it might work out for us since we have a pretty low frame
rate. I'd rather keep it like this though.

[Felix Dahlke, 2013/07/22 15:16:45]

But I definitely prefer how you approached the delay between animations to how I
did it in patch set 4! Not sure why I ended up using a second timer...

[Wladimir Palant, 2013/07/23 07:25:04]

You don't have a smooth animation right now, exactly because what the time
adjustment doesn't really work. So I'd rather prefer you switch to a simpler
approach that should also provide better animations.

[Felix Dahlke, 2013/07/23 10:38:08]

Implemented this as you prefer it, since it's not really important enough to
argue a lot about.

The good thing with this approach is that it's easier to understand, and at our
low frame rate and since this is just fading, not moving, it works well enough.
It does however stutter a bit now, and that's going to be worse on slower
systems. We cannot enforce a fixed frame rate. Then again, it doesn't really
look brilliant on slow systems even if we adjust for the variable frame rate
properly.

-
+      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});
-        iconAnimationTimer = setTimeout(animationStep, animationInterval);
-      });
+      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()
   {
+    var tab = animatedIconTab;
     stopIconAnimation();
     activeNotification = null;
+    refreshIconAndContextMenu(tab);
   };
 
-  startInformationNotificationAnimation();
-}
-
-function showCriticalNotification()
-{
-  var notification = webkitNotifications.createHTMLNotification("notification.html");
-  notification.show();
+  chrome.windows.getLastFocused({populate: true}, function(window)
+  {
+    chrome.tabs.query({active: true, windowId: window.id}, function(tabs)
+    {
+      tabs.forEach(refreshIconAndContextMenu);
+    });
+  });
 }
 
 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(refreshIconAndContextMenu);
-  });
+    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);