Commit e4822b9d authored by Mike Frysinger's avatar Mike Frysinger Committed by Commit Bot

grit: assume strings with util.TempDir inputs by default

All callers of this API pass in string inputs as their data.
Change the file mode to match as Python 3 enforces this.  We
make it an argument so callers can change this (which we will
need in the future to write binary data).

Because the Chrome CQ can't handle atomic sets, we squash in:

grit: chrome_scaled_image: use bytes for all png data

This code has been mixing bytes & strings when constructing the png
test images which Python 3 doesn't allow.  Clean it up to use bytes
for everything and write out the tempfiles in binary mode.

Bug: 983071
Test: `./grit/test_suite_all.py` passes
Change-Id: I1c71bf3305b9a49faff79a49ec68e64bbbd6135a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1757315
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738995}
parent fee504c6
...@@ -16,7 +16,7 @@ from grit import util ...@@ -16,7 +16,7 @@ from grit import util
from grit.gather import interface from grit.gather import interface
_PNG_SCALE_CHUNK = '\0\0\0\0csCl\xc1\x30\x60\x4d' _PNG_SCALE_CHUNK = b'\0\0\0\0csCl\xc1\x30\x60\x4d'
def _RescaleImage(data, from_scale, to_scale): def _RescaleImage(data, from_scale, to_scale):
...@@ -29,19 +29,19 @@ def _RescaleImage(data, from_scale, to_scale): ...@@ -29,19 +29,19 @@ def _RescaleImage(data, from_scale, to_scale):
return data return data
_PNG_MAGIC = '\x89PNG\r\n\x1a\n' _PNG_MAGIC = b'\x89PNG\r\n\x1a\n'
'''Mandatory first chunk in order for the png to be valid.''' '''Mandatory first chunk in order for the png to be valid.'''
_FIRST_CHUNK = 'IHDR' _FIRST_CHUNK = b'IHDR'
'''Special chunks to move immediately after the IHDR chunk. (so that the PNG '''Special chunks to move immediately after the IHDR chunk. (so that the PNG
remains valid.) remains valid.)
''' '''
_SPECIAL_CHUNKS = frozenset('csCl npTc'.split()) _SPECIAL_CHUNKS = frozenset(b'csCl npTc'.split())
'''Any ancillary chunk not in this list is deleted from the PNG.''' '''Any ancillary chunk not in this list is deleted from the PNG.'''
_ANCILLARY_CHUNKS_TO_LEAVE = frozenset( _ANCILLARY_CHUNKS_TO_LEAVE = frozenset(
'bKGD cHRM gAMA iCCP pHYs sBIT sRGB tRNS acTL fcTL fdAT'.split()) b'bKGD cHRM gAMA iCCP pHYs sBIT sRGB tRNS acTL fcTL fdAT'.split())
def _MoveSpecialChunksToFront(data): def _MoveSpecialChunksToFront(data):
...@@ -53,14 +53,14 @@ def _MoveSpecialChunksToFront(data): ...@@ -53,14 +53,14 @@ def _MoveSpecialChunksToFront(data):
rest = [] rest = []
for chunk in _ChunkifyPNG(data): for chunk in _ChunkifyPNG(data):
type = chunk[4:8] type = chunk[4:8]
critical = type < 'a' critical = type < b'a'
if type == _FIRST_CHUNK: if type == _FIRST_CHUNK:
first.append(chunk) first.append(chunk)
elif type in _SPECIAL_CHUNKS: elif type in _SPECIAL_CHUNKS:
special_chunks.append(chunk) special_chunks.append(chunk)
elif critical or type in _ANCILLARY_CHUNKS_TO_LEAVE: elif critical or type in _ANCILLARY_CHUNKS_TO_LEAVE:
rest.append(chunk) rest.append(chunk)
return ''.join(first + special_chunks + rest) return b''.join(first + special_chunks + rest)
def _ChunkifyPNG(data): def _ChunkifyPNG(data):
......
...@@ -34,21 +34,23 @@ _OUTFILETYPES = [ ...@@ -34,21 +34,23 @@ _OUTFILETYPES = [
_PNG_HEADER = ( _PNG_HEADER = (
'\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52' b'\x89\x50\x4e\x47\x0d\x0a\x1a\x0a\x00\x00\x00\x0d\x49\x48\x44\x52'
'\x00\x00\x00\x01\x00\x00\x00\x01\x08\x02\x00\x00\x00\x90\x77\x53' b'\x00\x00\x00\x01\x00\x00\x00\x01\x08\x02\x00\x00\x00\x90\x77\x53'
'\xde') b'\xde')
_PNG_FOOTER = ( _PNG_FOOTER = (
'\x00\x00\x00\x0c\x49\x44\x41\x54\x18\x57\x63\xf8\xff\xff\x3f\x00' b'\x00\x00\x00\x0c\x49\x44\x41\x54\x18\x57\x63\xf8\xff\xff\x3f\x00'
'\x05\xfe\x02\xfe\xa7\x35\x81\x84\x00\x00\x00\x00\x49\x45\x4e\x44' b'\x05\xfe\x02\xfe\xa7\x35\x81\x84\x00\x00\x00\x00\x49\x45\x4e\x44'
'\xae\x42\x60\x82') b'\xae\x42\x60\x82')
def _MakePNG(chunks): def _MakePNG(chunks):
pack_int32 = struct.Struct('>i').pack # Python 3 changed the return value of zlib.crc32 to an unsigned int.
format = 'i' if sys.version_info.major < 3 else 'I'
pack_int32 = struct.Struct('>' + format).pack
chunks = [pack_int32(len(payload)) + type + payload + chunks = [pack_int32(len(payload)) + type + payload +
pack_int32(zlib.crc32(type + payload)) pack_int32(zlib.crc32(type + payload))
for type, payload in chunks] for type, payload in chunks]
return _PNG_HEADER + ''.join(chunks) + _PNG_FOOTER return _PNG_HEADER + b''.join(chunks) + _PNG_FOOTER
def _GetFilesInPak(pakname): def _GetFilesInPak(pakname):
...@@ -95,7 +97,7 @@ def _RunBuildTest(self, structures, inputs, expected_outputs, skip_rc=False, ...@@ -95,7 +97,7 @@ def _RunBuildTest(self, structures, inputs, expected_outputs, skip_rc=False,
for context in expected_outputs) for context in expected_outputs)
infiles = { infiles = {
'in/in.grd': '''<?xml version="1.0" encoding="UTF-8"?> 'in/in.grd': ('''<?xml version="1.0" encoding="UTF-8"?>
<grit latest_public_release="0" current_release="1"> <grit latest_public_release="0" current_release="1">
<outputs> <outputs>
%s %s
...@@ -104,14 +106,15 @@ def _RunBuildTest(self, structures, inputs, expected_outputs, skip_rc=False, ...@@ -104,14 +106,15 @@ def _RunBuildTest(self, structures, inputs, expected_outputs, skip_rc=False,
%s %s
</release> </release>
</grit> </grit>
''' % (outputs, structures), ''' % (outputs, structures)).encode('utf-8'),
} }
for pngpath, pngdata in inputs.items(): for pngpath, pngdata in inputs.items():
normpath = os.path.normpath('in/' + pngpath) normpath = os.path.normpath('in/' + pngpath)
infiles[normpath] = pngdata infiles[normpath] = pngdata
class Options(object): class Options(object):
pass pass
with util.TempDir(infiles) as tmp_dir:
with util.TempDir(infiles, mode='wb') as tmp_dir:
with tmp_dir.AsCurrentDir(): with tmp_dir.AsCurrentDir():
options = Options() options = Options()
options.input = tmp_dir.GetPath('in/in.grd') options.input = tmp_dir.GetPath('in/in.grd')
...@@ -129,9 +132,9 @@ def _RunBuildTest(self, structures, inputs, expected_outputs, skip_rc=False, ...@@ -129,9 +132,9 @@ def _RunBuildTest(self, structures, inputs, expected_outputs, skip_rc=False,
class ChromeScaledImageUnittest(unittest.TestCase): class ChromeScaledImageUnittest(unittest.TestCase):
def testNormalFallback(self): def testNormalFallback(self):
d123a = _MakePNG([('AbCd', '')]) d123a = _MakePNG([(b'AbCd', b'')])
t123a = _MakePNG([('EfGh', '')]) t123a = _MakePNG([(b'EfGh', b'')])
d123b = _MakePNG([('IjKl', '')]) d123b = _MakePNG([(b'IjKl', b'')])
_RunBuildTest(self, _RunBuildTest(self,
_Structures(None, _Structures(None,
_Structure('IDR_A', 'a.png'), _Structure('IDR_A', 'a.png'),
...@@ -146,19 +149,19 @@ class ChromeScaledImageUnittest(unittest.TestCase): ...@@ -146,19 +149,19 @@ class ChromeScaledImageUnittest(unittest.TestCase):
}) })
def testNormalFallbackFailure(self): def testNormalFallbackFailure(self):
self.assertRaises(exception.FileNotFound, self.assertRaises(
_RunBuildTest, self, exception.FileNotFound, _RunBuildTest, self,
_Structures(None, _Structures(
_Structure('IDR_A', 'a.png'), None,
), _Structure('IDR_A', 'a.png'),
{'default_100_percent/a.png': _MakePNG([('AbCd', '')]), ), {
'tactile_100_percent/a.png': _MakePNG([('EfGh', '')]), 'default_100_percent/a.png': _MakePNG([(b'AbCd', b'')]),
}, 'tactile_100_percent/a.png': _MakePNG([(b'EfGh', b'')]),
{'tactile_123_percent': 'should fail before using this'}) }, {'tactile_123_percent': 'should fail before using this'})
def testLowresFallback(self): def testLowresFallback(self):
png = _MakePNG([('Abcd', '')]) png = _MakePNG([(b'Abcd', b'')])
png_with_csCl = _MakePNG([('csCl', ''),('Abcd', '')]) png_with_csCl = _MakePNG([(b'csCl', b''), (b'Abcd', b'')])
for outer in (None, False, True): for outer in (None, False, True):
for inner in (None, False, True): for inner in (None, False, True):
args = ( args = (
...@@ -185,9 +188,9 @@ class ChromeScaledImageUnittest(unittest.TestCase): ...@@ -185,9 +188,9 @@ class ChromeScaledImageUnittest(unittest.TestCase):
{'tactile_123_percent': 'should fail before using this'}) {'tactile_123_percent': 'should fail before using this'})
def testNoFallbackToDefaultLayout(self): def testNoFallbackToDefaultLayout(self):
d123a = _MakePNG([('AbCd', '')]) d123a = _MakePNG([(b'AbCd', b'')])
t123a = _MakePNG([('EfGh', '')]) t123a = _MakePNG([(b'EfGh', b'')])
d123b = _MakePNG([('IjKl', '')]) d123b = _MakePNG([(b'IjKl', b'')])
_RunBuildTest(self, _RunBuildTest(self,
_Structures(None, _Structures(None,
_Structure('IDR_A', 'a.png'), _Structure('IDR_A', 'a.png'),
......
...@@ -651,7 +651,8 @@ class TempDir(object): ...@@ -651,7 +651,8 @@ class TempDir(object):
'''Creates files with the specified contents in a temporary directory, '''Creates files with the specified contents in a temporary directory,
for unit testing. for unit testing.
''' '''
def __init__(self, file_data):
def __init__(self, file_data, mode='w'):
self._tmp_dir_name = tempfile.mkdtemp() self._tmp_dir_name = tempfile.mkdtemp()
assert not os.listdir(self.GetPath()) assert not os.listdir(self.GetPath())
for name, contents in file_data.items(): for name, contents in file_data.items():
...@@ -659,7 +660,7 @@ class TempDir(object): ...@@ -659,7 +660,7 @@ class TempDir(object):
dir_path = os.path.split(file_path)[0] dir_path = os.path.split(file_path)[0]
if not os.path.exists(dir_path): if not os.path.exists(dir_path):
os.makedirs(dir_path) os.makedirs(dir_path)
with open(file_path, 'wb') as f: with open(file_path, mode) as f:
f.write(file_data[name]) f.write(file_data[name])
def __enter__(self): def __enter__(self):
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment