Generate notification icon animations on the fly

[kzar]

Background

When Adblock Plus receives a notification the toolbar icon displays an animation to get the user's attention. The animation displayed depends on the type of notification received. The frame images for the animations are currently generated at build time, the extension then simply changes the icon URL repeatedly to create the animation effect.

What to change

We have decided it would be better to generate the images for the animations on the fly as they are displayed, instead of at build time. This will reduce the extension's size and reduce complexity of our build process.

• remove the code to generate these images at build time
• add code so that they are generated on the fly, when they are displayed at run time
• disable the animations for Safari

Hints for testers

• Make sure that the icon animates as smoothly as it did on Chrome/Opera with release previous versions.
• Make sure that the animation doesn't produce any more CPU load as it did before.
• Test whitelisting a page, make sure the icon is greyed in Chrome/Opera.
• Try a notification icon animation when the icon is greyed.
• Test a complete notification, including the dialog(s). Make sure all the images still render properly as this logic was slightly changed.
• In Safari ensure that the icon displays as before. Test a notification animation, make sure that the icon simply flicks between the normal Adblock Plus icon and the notification icon without blending.
• Make sure the animation plays properly when switching tabs and with multiple browser windows.
• Try playing one animation in the middle of another, make sure the first one finishes and then the next one plays.
• Try stopping an animation as it's playing, make sure it finishes smoothly and then stops.
• Try closing a window that has the animation playing, make sure an exception isn't logged in the background page.

[greiner]

It's not clear from the ticket description whether the images should be generated once and reused or generated throughout the animation. Latter could cause high CPU usage which should generally be avoided.

Apart from that, is there anything we intend to replace this with on Safari?

[kzar]

On Safari the first an final image will show, but not the blend in between. That will still have the effect of notifying the user. (Alternatively we could always display the standard icon / notification icon, but I don't think that would catch the user's eye in the same way.)

As for caching the blended images, good point. Once I have something working I guess we can decide how important it is.

[sebastian]

Replying to greiner:

It's not clear from the ticket description whether the images should be generated once and reused or generated throughout the animation. Latter could cause high CPU usage which should generally be avoided.

Note that, before we added support for Safari, we already generated the animation frames on the fly and it performed well enough. And we didn't cache the generated image data. But while on it, I agree, that it makes sense to do so. However, this is an implementation detail that doesn't necessarily has to be specified in the issue. But should rather be considered during implementation and code review.

Apart from that, is there anything we intend to replace this with on Safari?

No. The disadvantages just don't justify a solution that allow us to animate the icon on Safari. Unfortunately, on Safari the icon cannot be dynamically rendered. It has to be a file that is in the extension bundle.

However, we never showed a notification to any Safari users, and given the small amount of users we have on Safari (compared to other platforms) not having the icon animated there should be fine. And for urgency notifications we can still show desktop notifications to Safari users.

[sebastian]

Replying to kzar:

On Safari the first an final image will show, but not the blend in between. That will still have the effect of notifying the user.

I assumed that we simply keep showing the default icon on Safari. But indeed, we could simply show the notification icon instead, or cycle between them (without a blend effect). I don't have a strong opinion here. But I guess I would prefer whatever would result in the simplest implementation. As I indicated above, us using this mechanism on Safari is a rather theoretical scenario, anyway.

[kzar]

I went with showing the notification icon in Safari if the opacity is > 0.5. That's as close as we can get without blending.

[kzar]

​https://hg.adblockplus.org/adblockpluschrome/rev/ebcdcd485f8a
​https://hg.adblockplus.org/buildtools/rev/c8b5bb0aea6e

[sebastian]

I found that the icon animation consumes now quite some CPU on my notebook. Also you introduced a minor memory leak. I've uploaded a ​patch for review, with more details in there.

[sebastian]

I also found following corner cases:

• When the animation is stopped during transition the current frame persists
• Stopping the animation while loading the assets are loading doesn't have any effect, the animation will still run.
• When the icon animation is started again while already running, the transition will interrupt and start over.

Here is ​another patch fixing those.

[sebastian]

​https://hg.adblockplus.org/adblockpluschrome/rev/6c624674d9b2
​https://hg.adblockplus.org/adblockpluschrome/rev/15bf005a331a

[sebastian]

​https://hg.adblockplus.org/adblockpluschrome/rev/0ab1aa30dbc5
​https://hg.adblockplus.org/adblockpluschrome/rev/6639b13694ac
​https://hg.adblockplus.org/adblockpluschrome/rev/0fa95e10e016

[Ross]

Tested animations on a range of machines including i7 notebook, relatively old i5 desktop and an ancient 1.2Ghz Intel Atom machine. Only the Atom showed a noticeable CPU hit of about ~10% (from 2% idle to "12%") for 1-2 seconds (as the animations switches/blends) which seems fine for the age of the machine and what it's running (modern Chrome). It didn't lag out the machine or browser at all.

Safari behaves as expected/described, no blend, animation switch works. Notifications work.

Animations looked to behave correctly when switching tabs and I didn't notice any errors when closing the browser with the notification/animation active. Icon blends correctly to gray icon when on a whitelisted site. Have not tested "interrupt an animation with another" as I'm testing by rewriting the request made 1min after startup to a test notification.json.

Safari 9.0.1 / OS X 10.9.5
Chrome 48.0.2564.82 / Windows XP / Windows 8 / OS X 10.9.5 / Ubuntu 14.04
Opera 34.0.2036.50 / Windows 8 / Ubuntu 14.04