Update libpng to 1.6.22

third_party/libpng/libpng.gyp

 # Copyright (c) 2012 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.
 
 {
   'targets': [
     {
       'target_name': 'libpng',
       'dependencies': [
         '../zlib/zlib.gyp:zlib',
       ],
       'variables': {
         # Upstream uses self-assignment to avoid warnings.
-        'clang_warning_flags': [ '-Wno-self-assign' ]
+        'clang_warning_flags': [
+          '-Wno-self-assign',

[Nico, 2016/06/03 20:27:45]

why does the gn file have that tautological-compare thing but this file doesn't?

[msarett, 2016/06/03 21:26:01]

Possibly because we don't have a Windows/clang/gyp bot?  Do you think it makes
sense to have the flag here, even if it's unnecessary?

[Nico, 2016/06/03 21:36:49]

Ah, maybe that's the reason. You can send a try job to win_clang_x64_rel to get
a 64-bit gyp clang/win try result (it's backed by just 3 slaves and doesn't use
goma, so it's slow)

[msarett, 2016/06/06 14:18:15]

Sent a tryjob, will follow up on this.

[msarett, 2016/06/07 20:06:43]

The flag is not needed on win_clang_x64_rel.  Maybe win-clang-gn just has a
particular version of clang?

Anyway, I think this is fine as is (without the no-tautological-compare).

+        ],
       },
-      'defines': [
-        'CHROME_PNG_WRITE_SUPPORT',
-        'PNG_USER_CONFIG',
-      ],
       'sources': [
         'png.c',
-        'png.h',
-        'pngconf.h',
         'pngerror.c',
-        'pnggccrd.c',
         'pngget.c',
         'pngmem.c',
         'pngpread.c',
         'pngread.c',
         'pngrio.c',
         'pngrtran.c',
         'pngrutil.c',
         'pngset.c',
         'pngtrans.c',
-        'pngusr.h',
-        'pngvcrd.c',
         'pngwio.c',
         'pngwrite.c',
         'pngwtran.c',
         'pngwutil.c',
       ],
       'direct_dependent_settings': {
         'include_dirs': [
           '.',
         ],
-        'defines': [
-          'CHROME_PNG_WRITE_SUPPORT',
-          'PNG_USER_CONFIG',
-        ],
       },
       'export_dependent_settings': [
         '../zlib/zlib.gyp:zlib',
       ],
       # TODO(jschuh): http://crbug.com/167187
       'msvs_disabled_warnings': [ 4267 ],
       'conditions': [
+        # Disable ARM optimizations on IOS.  Can't find a way to get gyp to even try
+        # to compile the optimization files.  This works fine on GN.

[Nico, 2016/06/03 20:27:45]

what's the error if you don't do this?

[msarett, 2016/06/03 21:26:01]

Seeing a lot of this on ios_device:
Undefined symbols for architecture armv7:
  "_wk_png_init_filter_functions_neon", referenced from:
      _wk_png_read_filter_row in libpng.a(libpng.pngrutil.o)
ld: symbol(s) not found for architecture armv7
clang: error: linker command failed with exit code 1 (use -v to see invocation)

That symbol is defined in the arm/* files.  Patch Set 6 makes a fairly blatant
attempt to get ios to compile these files, but no luck.  I also tried some
things with "target_subarch" which similarly didn't work.  I'm happy to file a
bug.  How close are we to only caring about GN (not GYP)?

[Nico, 2016/06/03 21:36:49]

Hm, that sounds like maybe the prefix header isn't set on ios for some  reason
when building the arm files -- how is the prefix header supposed to be included
there?

[msarett, 2016/06/06 14:18:15]

What makes you think it's the prefix header?  FWIW, pngprefix.h is included from
pngpriv.h and pnglibconf.h and is not platform specific.

Additionally, looking at the build output, it appears that are arm/arm_init.c
and arm/filter_neon_intrinsics.c are not compiled with the rest of the png
sources.

[1/659] CC obj/third_party/libpng/libpng.pngrio.o
[2/659] CC obj/third_party/libpng/libpng.pngmem.o
[3/659] CC obj/third_party/libpng/libpng.pngget.o
[4/659] CC obj/third_party/libpng/libpng.pngwio.o
[5/659] CC obj/third_party/libpng/libpng.pngread.o
[6/659] CC obj/third_party/libpng/libpng.pngerror.o
[7/659] CC obj/third_party/libpng/libpng.pngtrans.o
[8/659] CC obj/third_party/libpng/libpng.pngwtran.o
[9/659] CC obj/third_party/libpng/libpng.pngset.o
[10/659] CC obj/third_party/libpng/libpng.pngpread.o
[11/659] CC obj/third_party/libpng/libpng.pngwrite.o
[12/659] CC obj/third_party/libpng/libpng.pngwutil.o
[13/659] CC obj/third_party/libpng/libpng.png.o
[14/659] CXX obj/third_party/skia/src/fonts/skia_library.SkRemotableFontMgr.o
[15/659] CXX obj/third_party/skia/src/opts/skia_opts_none.SkBlitRow_opts_none.o
[16/659] CXX obj/third_party/skia/src/opts/skia_opts_none.SkBlitMask_opts_none.o
[17/659] CC obj/third_party/libpng/libpng.pngrtran.o
[18/659] CC obj/third_party/libpng/libpng.pngrutil.o
[19/659] CXX obj/third_party/skia/src/ports/skia_library.SkTLS_pthread.o
[20/659] CXX
obj/third_party/skia/src/opts/skia_opts_none.SkBitmapProcState_opts_none.o
....

They are compiled on GN, which works fine.
[132/445] CC obj/third_party/libpng/libpng_sources/filter_neon_intrinsics.o
[135/445] CC obj/third_party/libpng/libpng_sources/arm_init.o

....

I originally thought it was likely a GYP bug, but I am further confused by the
compile output for ios on the recent (working) patch sets.

[9/958] CC obj/third_party/libpng/contrib/intel/libpng.intel_init.o
[10/958] CC obj/third_party/libpng/contrib/intel/libpng.filter_sse2_intrinsics.o

It's actually compiling the x86/64 optimizations (even though IOS is ARM?). 
Maybe this is because the host device is x86/64?  Still I don't get it.  But I
was happy enough with the green build.

---------------------------------

Any thoughts on this?  At the least, I think I should file a bug.

[msarett, 2016/06/07 20:06:43]

Filed https://bugs.chromium.org/p/chromium/issues/detail?id=618061

+        [ 'OS=="ios"', {
+          'defines': [
+            'PNG_ARM_NEON_OPT=0',
+          ],
+        }],
+
+        # SSE optimizations
+        [ 'target_arch=="ia32" or target_arch=="x64"', {
+          'defines': [
+            'PNG_INTEL_SSE_OPT=1',
+          ],
+          'sources': [
+            'contrib/intel/intel_init.c',
+            'contrib/intel/filter_sse2_intrinsics.c',
+          ],
+        }],
+
+        # ARM optimizations
+        [ 'target_arch=="arm" or target_arch=="arm64" and OS!="ios"', {
+          'defines': [
+            'PNG_ARM_NEON_OPT=2',
+            'PNG_ARM_NEON_IMPLEMENTATION=1',
+          ],
+          'sources': [
+            'arm/arm_init.c',
+            'arm/filter_neon_intrinsics.c',

[Nico, 2016/06/03 21:36:49]

btw, don't you have to add the .S file here?

[msarett, 2016/06/06 14:18:16]

filter_neon_intrinsics.c and filter_neon.S implement the same functions (one
with intrinsics, one with assembly).  The performance is virtually identical.

The intrinsics work on any Arm platform with Neon, while the assembly only works
on some Arm32 devices.  Additionally the maintainers have said that the assembly
is deprecated - though it doesn't seem to be formally deprecated.

I think it makes sense for us to always to compile and use the intrinsics when
on Arm, which is controlled by PNG_ARM_NEON_IMPLEMENTATION=1.

+          ],
+        }],
+      
         ['OS!="win"', {'product_name': 'png'}],
         ['OS=="win"', {
           'type': '<(component)',
         }, {
           # Chromium libpng does not support building as a shared_library
           # on non-Windows platforms.
           'type': 'static_library',
         }],
         ['OS=="win" and component=="shared_library"', {
           'defines': [
             'PNG_BUILD_DLL',
             'PNG_NO_MODULEDEF',
           ],
           'direct_dependent_settings': {
             'defines': [
               'PNG_USE_DLL',
             ],
           },
         }],
-        ['OS=="android" or chromecast==1', {
-          'conditions': [
-            ['OS=="android"', {
-              'toolsets': ['target', 'host'],
-            }],
-          ],
-          'defines': [
-            'CHROME_PNG_READ_PACK_SUPPORT',  # Required by freetype.
-          ],
-          'direct_dependent_settings': {
-            'defines': [
-              'CHROME_PNG_READ_PACK_SUPPORT',
-            ],
-          },
+        ['OS=="android"', {
+          'toolsets': ['target', 'host'],

[Nico, 2016/06/03 20:27:45]

why is this no longer needed for chromecast==1?

[msarett, 2016/06/03 21:26:01]

If you look closely at the base, these were only ever needed for Android.  The
difference is that we now enable PNG_READ_PACK_SUPPORT all the time.  IMO,
there's no reason to have different configs for different platforms.

[Nico, 2016/06/03 21:36:49]

Ah, thanks.

         }],
       ],
     },
   ]
 }