Merge pull request #7661 from diox/crush-developer-icons-and-previews

Run new image uploads through pngcrush, add a command to handle existing ones

.travis.yml

@@ -42,6 +42,7 @@ addons:
       - elasticsearch
       - gettext
       - librsvg2-bin
+      - pngcrush
 
 before_install:
   - mysql -e 'create database olympia;'

Dockerfile

@@ -40,6 +40,8 @@ RUN apt-get update && apt-get install -y \
         gettext \
         # Use rsvg-convert to render our static theme previews
         librsvg2-bin \
+        # Use pngcrush to optimize the PNGs uploaded by developers
+        pngcrush \
     && rm -rf /var/lib/apt/lists/*
 
 # Compile required locale

Dockerfile.deploy

@@ -44,6 +44,8 @@ RUN apt-get update && apt-get install -y \
         gettext                          \
         # Use rsvg-convert to render our static theme previews
         librsvg2-bin                     \
+        # Use pngcrush to optimize the PNGs uploaded by developers
+        pngcrush                         \
     && rm -rf /var/lib/apt/lists/*
 
 # Compile required locale

settings_test.py

@@ -76,6 +76,10 @@ LOGGING = {
     'loggers': {}
 }
 
+# To speed tests up, crushing uploaded images is disabled in tests except
+# where we explicitly want to test pngcrush.
+PNGCRUSH_BIN = '/bin/true'
+
 
 ###############################################################################
 # Only if running on a CI server.

src/olympia/addons/tasks.py

@@ -19,7 +19,8 @@ from olympia.amo.celery import task
 from olympia.amo.decorators import set_modified_on, write
 from olympia.amo.storage_utils import rm_stored_dir
 from olympia.amo.templatetags.jinja_helpers import user_media_path
-from olympia.amo.utils import ImageCheck, LocalFileStorage, cache_ns_key
+from olympia.amo.utils import (
+    ImageCheck, LocalFileStorage, cache_ns_key, pngcrush_image)
 from olympia.applications.models import AppVersion
 from olympia.files.utils import RDFExtractor, get_file, SafeZip
 from olympia.lib.es.utils import index_objects
@@ -162,6 +163,8 @@ def create_persona_preview_images(src, full_dst, **kw):
         i.load()
         with storage.open(full_dst[1], 'wb') as fp:
             i.save(fp, 'png')
+    pngcrush_image(full_dst[0])
+    pngcrush_image(full_dst[1])
     return True
 
 
@@ -177,6 +180,7 @@ def save_persona_image(src, full_dst, **kw):
         i = Image.open(fp)
         with storage.open(full_dst, 'wb') as fp:
             i.save(fp, 'png')
+    pngcrush_image(full_dst)
     return True
 
 

src/olympia/addons/tests/test_forms.py

@@ -355,5 +355,5 @@ class TestThemeForm(TestCase):
             'license': 1}, request=request)
         assert form.is_valid()
         # Make sure there's no database issue, like too long data for the
-        # author or display_sername fields.
+        # author or display_username fields.
         form.save()

src/olympia/addons/tests/test_tasks.py

@@ -0,0 +1,71 @@
+import mock
+import os
+import tempfile
+
+from django.conf import settings
+
+from olympia.addons.tasks import (
+    create_persona_preview_images, save_persona_image)
+from olympia.amo.tests import addon_factory, TestCase
+from olympia.amo.tests.test_helpers import get_image_path
+from olympia.amo.utils import image_size
+
+
+class TestPersonaImageFunctions(TestCase):
+    @mock.patch('olympia.addons.tasks.pngcrush_image')
+    def test_create_persona_preview_image(self, pngcrush_image_mock):
+        addon = addon_factory()
+        addon.modified = self.days_ago(41)
+        # Given an image, a 680x100 and a 32x32 thumbnails need to be generated
+        # and processed with pngcrush.
+        expected_dst1 = tempfile.NamedTemporaryFile(
+            mode='r+w+b', suffix=".png", delete=False, dir=settings.TMP_PATH)
+        expected_dst2 = tempfile.NamedTemporaryFile(
+            mode='r+w+b', suffix=".png", delete=False, dir=settings.TMP_PATH)
+        create_persona_preview_images(
+            src=get_image_path('persona-header.jpg'),
+            full_dst=[expected_dst1.name, expected_dst2.name],
+            set_modified_on=[addon],
+        )
+        # pngcrush_image should have been called twice, once for each
+        # destination thumbnail.
+        assert pngcrush_image_mock.call_count == 2
+        assert pngcrush_image_mock.call_args_list[0][0][0] == (
+            expected_dst1.name)
+        assert pngcrush_image_mock.call_args_list[1][0][0] == (
+            expected_dst2.name)
+
+        assert image_size(expected_dst1.name) == (680, 100)
+        assert image_size(expected_dst2.name) == (32, 32)
+
+        addon.reload()
+        self.assertCloseToNow(addon.modified)
+
+    @mock.patch('olympia.addons.tasks.pngcrush_image')
+    def test_save_persona_image(self, pngcrush_image_mock):
+        # save_persona_image() simply saves an image as a png to the
+        # destination file. The image should be processed with pngcrush.
+        expected_dst = tempfile.NamedTemporaryFile(
+            mode='r+w+b', suffix=".png", delete=False, dir=settings.TMP_PATH)
+        save_persona_image(
+            get_image_path('persona-header.jpg'),
+            expected_dst.name
+        )
+        # pngcrush_image should have been called once.
+        assert pngcrush_image_mock.call_count == 1
+        assert pngcrush_image_mock.call_args_list[0][0][0] == expected_dst.name
+
+    @mock.patch('olympia.addons.tasks.pngcrush_image')
+    def test_save_persona_image_not_an_image(self, pngcrush_image_mock):
+        # If the source is not an image, save_persona_image() should just
+        # return early without writing the destination or calling pngcrush.
+        expected_dst = tempfile.NamedTemporaryFile(
+            mode='r+w+b', suffix=".png", delete=False, dir=settings.TMP_PATH)
+        save_persona_image(
+            get_image_path('non-image.png'),
+            expected_dst.name
+        )
+        # pngcrush_image should not have been called.
+        assert pngcrush_image_mock.call_count == 0
+        # the destination file should not have been written to.
+        assert os.stat(expected_dst.name).st_size == 0

src/olympia/amo/tests/test_utils_.py

@@ -13,7 +13,8 @@ from olympia import amo
 from olympia.addons.models import Addon
 from olympia.amo.tests import TestCase, addon_factory
 from olympia.amo.utils import (
-    attach_trans_dict, get_locale_from_lang, translations_for_field, walkfiles)
+    attach_trans_dict, get_locale_from_lang, pngcrush_image,
+    translations_for_field, walkfiles)
 from olympia.versions.models import Version
 
 
@@ -203,3 +204,23 @@ def test_get_locale_from_lang(lang):
 def test_bidi_language_in_amo_languages(lang):
     """Make sure all bidi marked locales are in AMO_LANGUAGES too."""
     assert lang in settings.AMO_LANGUAGES or lang in settings.DEBUG_LANGUAGES
+
+
+@mock.patch('olympia.amo.utils.subprocess')
+def test_pngcrush_image(subprocess_mock):
+    subprocess_mock.Popen.return_value.communicate.return_value = ('', '')
+    subprocess_mock.Popen.return_value.returncode = 0  # success
+    assert pngcrush_image('/tmp/some_file.png')
+    assert subprocess_mock.Popen.call_count == 1
+    assert subprocess_mock.Popen.call_args_list[0][0][0] == [
+        settings.PNGCRUSH_BIN, '-q', '-reduce', '-ow',
+        '/tmp/some_file.png', '/tmp/some_file.crush.png',
+    ]
+    assert subprocess_mock.Popen.call_args_list[0][1] == {
+        'stdout': subprocess_mock.PIPE,
+        'stderr': subprocess_mock.PIPE,
+    }
+
+    # Make sure that exceptions for this are silent.
+    subprocess_mock.Popen.side_effect = Exception
+    assert not pngcrush_image('/tmp/some_other_file.png')

src/olympia/amo/utils.py

@@ -16,6 +16,7 @@ import unicodedata
 import urllib
 import urlparse
 import string
+import subprocess
 
 import django.core.mail
 
@@ -477,6 +478,50 @@ def clean_nl(string):
     return serializer.render(stream)
 
 
+def image_size(filename):
+    """
+    Return an image size tuple, as returned by PIL.
+    """
+    with Image.open(filename) as img:
+        size = img.size
+    return size
+
+
+def pngcrush_image(src, **kw):
+    """
+    Optimizes a PNG image by running it through Pngcrush.
+    """
+    log.info('Optimizing image: %s' % src)
+    try:
+        # When -ow is used, the output file name (second argument after
+        # options) is used as a temporary filename (that must reside on the
+        # same filesystem as the original) to save the optimized file before
+        # overwriting the original. By default it's "pngout.png" but we want
+        # that to be unique in order to avoid clashes with multiple tasks
+        # processing different images in parallel.
+        tmp_path = '%s.crush.png' % os.path.splitext(src)[0]
+        # -brute is not recommended, and in general does not improve things a
+        # lot. -reduce is on by default for pngcrush above 1.8.0, but we're
+        # still on an older version (1.7.85 at the time of writing this
+        # comment, because that's what comes with Debian stretch that is used
+        # for our docker container).
+        cmd = [settings.PNGCRUSH_BIN, '-q', '-reduce', '-ow', src, tmp_path]
+        process = subprocess.Popen(
+            cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        stdout, stderr = process.communicate()
+
+        if process.returncode != 0:
+            log.error('Error optimizing image: %s; %s' % (src, stderr.strip()))
+            return False
+
+        log.info('Image optimization completed for: %s' % src)
+        return True
+
+    except Exception, e:
+        log.error('Error optimizing image: %s; %s' % (src, e))
+    return False
+
+
 def resize_image(source, destination, size=None):
     """Resizes and image from src, to dst.
     Returns a tuple of new width and height, original width and height.
@@ -496,7 +541,7 @@ def resize_image(source, destination, size=None):
             im = processors.scale_and_crop(im, size)
     with storage.open(destination, 'wb') as fp:
         im.save(fp, 'png')
-
+    pngcrush_image(destination)
     return (im.size, original_size)
 
 

src/olympia/devhub/management/commands/crush_images_for_top_addons.py

@@ -0,0 +1,98 @@
+# -*- coding: utf-8 -*-
+from django.core.management.base import BaseCommand
+
+from olympia import amo
+from olympia.addons.models import Addon, Persona
+from olympia.bandwagon.models import Collection, FeaturedCollection
+from olympia.constants.categories import CATEGORIES
+from olympia.devhub.tasks import (
+    pngcrush_existing_preview, pngcrush_existing_icons,
+    pngcrush_existing_theme)
+from olympia.discovery.data import discopane_items
+from olympia.users.models import UserProfile
+
+
+class Command(BaseCommand):
+    help = 'Optimize existing images for "top" add-ons.'
+
+    def add_arguments(self, parser):
+        """Handle command arguments."""
+        parser.add_argument(
+            '--dry-run',
+            action='store_true',
+            dest='dry_run',
+            default=False,
+            help='Do not really fire the tasks.')
+
+    def handle(self, *args, **options):
+        """Command entry point."""
+        self.dry_run = options.get('dry_run', False)
+        addons = self.fetch_addons()
+        if not self.dry_run:
+            self.crush_addons(addons)
+
+    def crush_addons(self, addons):
+        for addon in addons:
+            if addon.is_persona():
+                try:
+                    if addon.persona.is_new():
+                        pngcrush_existing_theme.delay(
+                            addon.persona.pk, set_modified_on=[addon])
+                except Persona.DoesNotExist:
+                    pass
+            else:
+                pngcrush_existing_icons.delay(
+                    addon.pk, set_modified_on=[addon])
+                for preview in addon.previews.all():
+                    pngcrush_existing_preview.delay(
+                        preview.pk, set_modified_on=[preview])
+
+    def fetch_addons(self):
+        """
+        Fetch the add-ons we want to optimize the images of. That'll be any
+        add-on directly present on one of the landing pages (category landing
+        pages, mozilla collections landing pages, homepage).
+        """
+        print 'Starting to fetch all addons...'
+        addons = set()
+
+        print 'Fetching featured add-ons.'
+        for featuredcollection in FeaturedCollection.objects.all():
+            addons.update(featuredcollection.collection.addons.all())
+
+        print 'Fetching mozilla collections add-ons.'
+        try:
+            mozilla = UserProfile.objects.get(username='mozilla')
+            for collection in Collection.objects.filter(author=mozilla):
+                addons.update(collection.addons.all())
+        except UserProfile.DoesNotExist:
+            print 'Skipping mozilla collections as user does not exist.'
+
+        print 'Fetching 5 top-rated extensions/themes from each category.'
+        for cat in CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION].values():
+            addons.update(Addon.objects.public().filter(
+                category=cat.id).order_by('-bayesian_rating')[:5])
+        for cat in CATEGORIES[amo.FIREFOX.id][amo.ADDON_PERSONA].values():
+            addons.update(Addon.objects.public().filter(
+                category=cat.id).order_by('-bayesian_rating')[:5])
+
+        print 'Fetching 5 trending extensions/themes from each category.'
+        for cat in CATEGORIES[amo.FIREFOX.id][amo.ADDON_EXTENSION].values():
+            addons.update(Addon.objects.public().filter(
+                category=cat.id).order_by('-hotness')[:5])
+        for cat in CATEGORIES[amo.FIREFOX.id][amo.ADDON_PERSONA].values():
+            addons.update(Addon.objects.public().filter(
+                category=cat.id).order_by('-hotness')[:5])
+
+        print 'Fetching 25 most popular themes.'
+        addons.update(
+            Addon.objects.public().filter(
+                type=amo.ADDON_PERSONA).order_by('-average_daily_users')[:25])
+
+        print 'Fetching disco pane add-ons.'
+        addons.update(
+            Addon.objects.public().filter(
+                id__in=[item.addon_id for item in discopane_items['default']]))
+
+        print 'Done fetching, %d add-ons to process total.' % len(addons)
+        return addons

src/olympia/devhub/tasks.py

@@ -26,16 +26,16 @@ import validator
 from celery.exceptions import SoftTimeLimitExceeded
 from celery.result import AsyncResult
 from django_statsd.clients import statsd
-from PIL import Image
 
 import olympia.core.logger
 
 from olympia import amo
-from olympia.addons.models import Addon
+from olympia.addons.models import Addon, Persona, Preview
 from olympia.amo.celery import task
 from olympia.amo.decorators import atomic, set_modified_on, write
 from olympia.amo.utils import (
-    resize_image, send_html_mail_jinja, utc_millesecs_from_epoch)
+    image_size, pngcrush_image, resize_image, send_html_mail_jinja,
+    utc_millesecs_from_epoch)
 from olympia.applications.management.commands import dump_apps
 from olympia.applications.models import AppVersion
 from olympia.files.models import File, FileUpload, FileValidation
@@ -619,6 +619,72 @@ def track_validation_stats(json_result, addons_linter=False):
                 .format(runner, listed_tag, result_kind))
 
 
+@task
+@write
+@set_modified_on
+def pngcrush_existing_icons(addon_id):
+    """
+    Call pngcrush_image() on the icons of a given add-on.
+    """
+    log.info('Crushing icons for add-on %s', addon_id)
+    addon = Addon.objects.get(pk=addon_id)
+    if addon.icon_type != 'image/png':
+        log.info('Aborting icon crush for add-on %s, icon type is not a PNG.',
+                 addon_id)
+        return
+    icon_dir = addon.get_icon_dir()
+    pngcrush_image(os.path.join(icon_dir, '%s-64.png' % addon_id))
+    pngcrush_image(os.path.join(icon_dir, '%s-32.png' % addon_id))
+    # Return an icon hash that set_modified_on decorator will set on the add-on
+    # after a small delay. This is normally done with the true md5 hash of the
+    # original icon, but we don't necessarily have it here. We could read one
+    # of the icons we modified but it does not matter just fake a hash to
+    # indicate it was "manually" crushed.
+    return {
+        'icon_hash': 'mcrushed'
+    }
+
+
+@task
+@write
+@set_modified_on
+def pngcrush_existing_preview(preview_id):
+    """
+    Call pngcrush_image() on the images of a given add-on Preview object.
+    """
+    log.info('Crushing images for Preview %s', preview_id)
+    preview = Preview.objects.get(pk=preview_id)
+    pngcrush_image(preview.thumbnail_path)
+    pngcrush_image(preview.image_path)
+    # We don't need a hash, previews are cachebusted with their modified date,
+    # which does not change often. @set_modified_on will do that for us
+    # automatically if the task was called with set_modified_on_obj=[preview].
+
+
+@task
+@write
+@set_modified_on
+def pngcrush_existing_theme(persona_id):
+    """
+    Call pngcrush_image() on the images of a given Persona object.
+    """
+    log.info('Crushing images for Persona %s', persona_id)
+    persona = Persona.objects.get(pk=persona_id)
+    # Only do this on "new" Personas with persona_id = 0, the older ones (with
+    # a persona_id) have jpeg and not pngs.
+    if not persona.is_new():
+        log.info('Aborting images crush for Persona %s (too old).', persona_id)
+        return
+    pngcrush_image(persona.preview_path)
+    # No need to crush thumb_path, it's the same as preview_path for "new"
+    # Personas.
+    pngcrush_image(persona.icon_path)
+    if persona.header:
+        pngcrush_image(persona.header_path)
+    if persona.footer:
+        pngcrush_image(persona.footer_path)
+
+
 @task
 @set_modified_on
 def resize_icon(source, dest_folder, target_sizes, **kw):
@@ -686,9 +752,8 @@ def get_preview_sizes(ids, **kw):
             try:
                 log.info('Getting size for preview: %s' % preview.pk)
                 sizes = {
-                    'thumbnail': Image.open(
-                        storage.open(preview.thumbnail_path)).size,
-                    'image': Image.open(storage.open(preview.image_path)).size,
+                    'thumbnail': image_size(preview.thumbnail_path),
+                    'image': image_size(preview.image_path),
                 }
                 preview.update(sizes=sizes)
             except Exception, err:

src/olympia/devhub/tests/test_commands.py

@@ -0,0 +1,102 @@
+import mock
+import os
+
+from django.core.management import call_command
+
+from olympia import amo
+from olympia.addons.models import Preview
+from olympia.amo.tests import addon_factory, TestCase
+from olympia.devhub.management.commands import crush_images_for_top_addons
+
+
+class TestCrushImagesForTopAddons(TestCase):
+    @mock.patch('olympia.devhub.tasks.pngcrush_image')
+    def test_crush_icons(self, pngcrush_image_mock):
+        addon1 = addon_factory(icon_type='image/png')
+        icon_dir = addon1.get_icon_dir()
+        crush_images_for_top_addons.Command().crush_addons([addon1])
+        assert pngcrush_image_mock.call_count == 2
+        assert pngcrush_image_mock.call_args_list[0][0][0] == os.path.join(
+            icon_dir, '%s-64.png' % addon1.pk)
+        assert pngcrush_image_mock.call_args_list[1][0][0] == os.path.join(
+            icon_dir, '%s-32.png' % addon1.pk)
+
+    @mock.patch('olympia.devhub.tasks.pngcrush_image')
+    def test_crush_nothing(self, pngcrush_image_mock):
+        addon1 = addon_factory()  # No previews or icons to crush here.
+        crush_images_for_top_addons.Command().crush_addons([addon1])
+        assert pngcrush_image_mock.call_count == 0
+
+    @mock.patch('olympia.devhub.tasks.pngcrush_image')
+    def test_crush_previews(self, pngcrush_image_mock):
+        addon1 = addon_factory()
+        preview1 = Preview.objects.create(addon=addon1)
+        crush_images_for_top_addons.Command().crush_addons([addon1])
+        assert pngcrush_image_mock.call_count == 2
+        assert pngcrush_image_mock.call_args_list[0][0][0] == (
+            preview1.thumbnail_path)
+        assert pngcrush_image_mock.call_args_list[1][0][0] == (
+            preview1.image_path)
+
+    @mock.patch('olympia.devhub.tasks.pngcrush_image')
+    def test_crush_new_but_weird_persona(self, pngcrush_image_mock):
+        addon1 = addon_factory(type=amo.ADDON_PERSONA)
+        persona = addon1.persona
+        persona.persona_id = 0
+        persona.save()
+        crush_images_for_top_addons.Command().crush_addons([addon1])
+        assert pngcrush_image_mock.call_count == 2
+        assert pngcrush_image_mock.call_args_list[0][0][0] == (
+            persona.preview_path)
+        assert pngcrush_image_mock.call_args_list[1][0][0] == (
+            persona.icon_path)
+
+    @mock.patch('olympia.devhub.tasks.pngcrush_image')
+    def test_crush_new_persona_with_headerfooter(self, pngcrush_image_mock):
+        addon1 = addon_factory(type=amo.ADDON_PERSONA)
+        persona = addon1.persona
+        persona.persona_id = 0
+        persona.header = 'header.png'
+        persona.footer = 'footer.png'
+        persona.save()
+        crush_images_for_top_addons.Command().crush_addons([addon1])
+        assert pngcrush_image_mock.call_count == 4
+        assert pngcrush_image_mock.call_args_list[0][0][0] == (
+            persona.preview_path)
+        assert pngcrush_image_mock.call_args_list[1][0][0] == (
+            persona.icon_path)
+        assert pngcrush_image_mock.call_args_list[2][0][0] == (
+            persona.header_path)
+        assert pngcrush_image_mock.call_args_list[3][0][0] == (
+            persona.footer_path)
+
+    @mock.patch('olympia.devhub.tasks.pngcrush_image')
+    def test_crush_old_persona(self, pngcrush_image_mock):
+        addon1 = addon_factory(type=amo.ADDON_PERSONA)
+        crush_images_for_top_addons.Command().crush_addons([addon1])
+        assert pngcrush_image_mock.call_count == 0
+
+    @mock.patch('olympia.devhub.tasks.pngcrush_image')
+    def test_full_run(self, pngcrush_image_mock):
+        addon1 = addon_factory(icon_type='image/png')
+        Preview.objects.create(addon=addon1)
+        Preview.objects.create(addon=addon1)
+        addon2 = addon_factory(type=amo.ADDON_PERSONA)
+        persona = addon2.persona
+        persona.persona_id = 0
+        persona.header = 'header.png'
+        persona.footer = 'footer.png'
+        persona.save()
+
+        call_command('crush_images_for_top_addons')
+        # 10 calls:
+        # - 2 icons sizes for the extension
+        # - 2 sizes for each of the 2 previews of the extension
+        # - 1 icon and 1 preview for the persona, plus 1 header and 1 footer.
+        assert pngcrush_image_mock.call_count == 10
+
+    @mock.patch('olympia.devhub.tasks.pngcrush_image')
+    def test_dry_run(self, pngcrush_image_mock):
+        addon_factory()
+        call_command('crush_images_for_top_addons', dry_run=True)
+        assert pngcrush_image_mock.call_count == 0

src/olympia/devhub/tests/test_forms.py

@@ -237,7 +237,8 @@ class TestPreviewForm(TestCase):
         form.save(addon)
         assert update_mock.called
 
-    def test_preview_size(self):
+    @mock.patch('olympia.amo.utils.pngcrush_image')
+    def test_preview_size(self, pngcrush_image_mock):
         addon = Addon.objects.get(pk=3615)
         name = 'non-animated.gif'
         form = forms.PreviewForm({'caption': 'test', 'upload_hash': name,
@@ -254,6 +255,12 @@ class TestPreviewForm(TestCase):
         assert os.path.exists(preview.thumbnail_path)
         assert os.path.exists(preview.original_path)
 
+        assert pngcrush_image_mock.call_count == 2
+        assert pngcrush_image_mock.call_args_list[0][0][0] == (
+            preview.thumbnail_path)
+        assert pngcrush_image_mock.call_args_list[1][0][0] == (
+            preview.image_path)
+
 
 class TestThemeForm(TestCase):
     fixtures = ['base/user_2519']

src/olympia/devhub/tests/test_tasks.py

@@ -22,7 +22,7 @@ from olympia.addons.models import Addon
 from olympia.amo.templatetags.jinja_helpers import user_media_path
 from olympia.amo.tests import TestCase, addon_factory, version_factory
 from olympia.amo.tests.test_helpers import get_addon_file, get_image_path
-from olympia.amo.utils import utc_millesecs_from_epoch
+from olympia.amo.utils import image_size, utc_millesecs_from_epoch
 from olympia.applications.models import AppVersion
 from olympia.constants.base import VALIDATOR_SKELETON_RESULTS
 from olympia.devhub import tasks
@@ -91,9 +91,12 @@ def _uploader(resize_size, final_size):
         assert src_image.size == original_size
         dest_name = os.path.join(uploadto, '1234')
 
-        return_value = tasks.resize_icon(src.name, dest_name, [rsize])
+        with mock.patch('olympia.amo.utils.pngcrush_image') as pngcrush_mock:
+            return_value = tasks.resize_icon(src.name, dest_name, [rsize])
         dest_image = '%s-%s.png' % (dest_name, rsize)
-        assert Image.open(open(dest_image)).size == expected_size
+        assert pngcrush_mock.call_count == 1
+        assert pngcrush_mock.call_args_list[0][0][0] == dest_image
+        assert image_size(dest_image) == expected_size
         # original should have been moved to -original
         orig_image = '%s-original.png' % dest_name
         assert os.path.exists(orig_image)

src/olympia/devhub/tests/test_views_edit.py

@@ -7,7 +7,6 @@ from django.db.models import Q
 
 import mock
 
-from PIL import Image
 from pyquery import PyQuery as pq
 
 from olympia import amo
@@ -20,6 +19,7 @@ from olympia.amo.tests import (
     TestCase, addon_factory, formset, initial, req_factory_factory)
 from olympia.amo.tests.test_helpers import get_image_path
 from olympia.amo.urlresolvers import reverse
+from olympia.amo.utils import image_size
 from olympia.bandwagon.models import (
     Collection, CollectionAddon, FeaturedCollection)
 from olympia.constants.categories import CATEGORIES_BY_ID
@@ -754,7 +754,7 @@ class TestEditMedia(BaseTestEdit):
 
         assert storage.exists(dest)
 
-        assert Image.open(storage.open(dest)).size == (32, 12)
+        assert image_size(dest) == (32, 12)
 
         assert addon.icon_type == 'image/png'
         assert addon.icon_hash == 'bb362450'
@@ -800,7 +800,7 @@ class TestEditMedia(BaseTestEdit):
 
         assert storage.exists(dest)
 
-        assert Image.open(storage.open(dest)).size == (48, 48)
+        assert image_size(dest) == (48, 48)
 
         assert addon.icon_type == 'image/png'
         assert addon.icon_hash == 'f02063c9'

src/olympia/lib/settings_base.py

@@ -81,6 +81,9 @@ UGLIFY_BIN = 'uglifyjs'  # Set as None to use YUI instead (at your risk).
 # rsvg-convert is used to save our svg static theme previews to png
 RSVG_CONVERT_BIN = 'rsvg-convert'
 
+# Path to pngcrush (to optimize the PNGs uploaded by developers).
+PNGCRUSH_BIN = 'pngcrush'
+
 FLIGTAR = 'amo-admins+fligtar-rip@mozilla.org'
 REVIEWERS_EMAIL = 'amo-editors@mozilla.org'
 THEMES_EMAIL = 'theme-reviews@mozilla.org'
@@ -1276,6 +1279,13 @@ CELERY_TASK_ROUTES = {
     # Github API
     'olympia.github.tasks.process_results': {'queue': 'devhub'},
     'olympia.github.tasks.process_webhook': {'queue': 'devhub'},
+
+    # Temporary tasks to crush existing images.
+    # Go in the addons queue to leave the 'devhub' queue free to process
+    # validations etc.
+    'olympia.devhub.tasks.pngcrush_existing_theme': {'queue': 'addons'},
+    'olympia.devhub.tasks.pngcrush_existing_preview': {'queue': 'addons'},
+    'olympia.devhub.tasks.pngcrush_existing_icons': {'queue': 'addons'},
 }
 
 

src/olympia/versions/tasks.py

@@ -14,6 +14,7 @@ import olympia.core.logger
 from olympia import amo
 from olympia.amo.celery import task
 from olympia.amo.decorators import write
+from olympia.amo.utils import pngcrush_image
 
 
 log = olympia.core.logger.getLogger('z.files.utils')
@@ -72,4 +73,5 @@ def generate_static_theme_preview(theme_manifest, header_root, preview):
     svg = tmpl.render(context).encode('utf-8')
     size = write_svg_to_png(svg, preview.image_path)
     if size:
+        pngcrush_image(preview.image_path)
         preview.update(sizes={'image': size})

src/olympia/versions/tests/test_tasks.py

@@ -43,6 +43,7 @@ def test_write_svg_to_png():
 
 
 @pytest.mark.django_db
+@mock.patch('olympia.versions.tasks.pngcrush_image')
 @mock.patch('olympia.versions.tasks.write_svg_to_png')
 @pytest.mark.parametrize(
     'header_url, header_height, preserve_aspect_ratio, mimetype', (
@@ -52,8 +53,8 @@ def test_write_svg_to_png():
     )
 )
 def test_generate_static_theme_preview(
-        write_svg_to_png, header_url, header_height, preserve_aspect_ratio,
-        mimetype):
+        write_svg_to_png, pngcrush_image_mock, header_url, header_height,
+        preserve_aspect_ratio, mimetype):
     write_svg_to_png.return_value = (123, 456)
     theme_manifest = {
         "images": {
@@ -72,7 +73,9 @@ def test_generate_static_theme_preview(
     addon = addon_factory()
     preview = Preview.objects.create(addon=addon)
     generate_static_theme_preview(theme_manifest, header_root, preview)
-    write_svg_to_png.assert_called()
+    write_svg_to_png.call_count == 1
+    assert pngcrush_image_mock.call_count == 1
+    assert pngcrush_image_mock.call_args_list[0][0][0] == preview.image_path
     ((svg_content, png_path), _) = write_svg_to_png.call_args
     assert png_path == preview.image_path
     # check header is there.