Commit 33155c5c authored by Samuel Huang's avatar Samuel Huang Committed by Commit Bot

[SuperSize] Fix file handle usage for .sizediff files.

Previously, SaveDeltaSizeInfo() and LoadDeltaSizeInfo() take an optional
|file_obj| parameter, and uses it in a "with" block. This causes the
inject |file_obj| to get closed. This side-effect is undesirable for the
usual usage pattern of callers lending |file_obj| to functions. This CL
fixes the issue by using a one-level recursion to create |file_obj| if
it's not provided.

Change-Id: I2132aea64bb29edd49576dbaa4b0797a6afb80f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425224
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810099}
parent 73ca0e7c
...@@ -627,7 +627,7 @@ def SaveSizeInfo(size_info, ...@@ -627,7 +627,7 @@ def SaveSizeInfo(size_info,
logging.debug('Serialization complete. Gzipping...') logging.debug('Serialization complete. Gzipping...')
with _OpenGzipForWrite(path, file_obj=file_obj) as f: with _OpenGzipForWrite(path, file_obj=file_obj) as f:
f.write(bytesio.getbuffer()) f.write(bytesio.getvalue())
def LoadSizeInfo(filename, file_obj=None): def LoadSizeInfo(filename, file_obj=None):
...@@ -639,6 +639,10 @@ def LoadSizeInfo(filename, file_obj=None): ...@@ -639,6 +639,10 @@ def LoadSizeInfo(filename, file_obj=None):
def SaveDeltaSizeInfo(delta_size_info, path, file_obj=None): def SaveDeltaSizeInfo(delta_size_info, path, file_obj=None):
"""Saves |delta_size_info| to |path|.""" """Saves |delta_size_info| to |path|."""
if not file_obj:
with open(path, 'wb') as f:
return SaveDeltaSizeInfo(delta_size_info, path, f)
changed_symbols = delta_size_info.raw_symbols \ changed_symbols = delta_size_info.raw_symbols \
.WhereDiffStatusIs(models.DIFF_STATUS_UNCHANGED).Inverted() .WhereDiffStatusIs(models.DIFF_STATUS_UNCHANGED).Inverted()
before_symbols = models.SymbolGroup( before_symbols = models.SymbolGroup(
...@@ -663,44 +667,46 @@ def SaveDeltaSizeInfo(delta_size_info, path, file_obj=None): ...@@ -663,44 +667,46 @@ def SaveDeltaSizeInfo(delta_size_info, path, file_obj=None):
include_padding=True, include_padding=True,
sparse_symbols=before_symbols) sparse_symbols=before_symbols)
with file_obj or open(path, 'wb') as output_file: w = _Writer(file_obj)
w = _Writer(output_file) w.WriteBytes(_COMMON_HEADER + _SIZEDIFF_HEADER)
w.WriteBytes(_COMMON_HEADER + _SIZEDIFF_HEADER) # JSON header fields
# JSON header fields fields = {
fields = { 'version': _SIZEDIFF_VERSION,
'version': _SIZEDIFF_VERSION, 'before_length': before_size_file.tell(),
'before_length': before_size_file.tell(), }
} fields_str = json.dumps(fields, indent=2, sort_keys=True)
fields_str = json.dumps(fields, indent=2, sort_keys=True)
w.WriteLine(str(len(fields_str))) w.WriteLine(str(len(fields_str)))
w.WriteLine(fields_str) w.WriteLine(fields_str)
w.WriteBytes(before_size_file.getbuffer()) w.WriteBytes(before_size_file.getvalue())
after_promise.get() after_promise.get()
w.WriteBytes(after_size_file.getbuffer()) w.WriteBytes(after_size_file.getvalue())
def LoadDeltaSizeInfo(filename, file_obj=None): def LoadDeltaSizeInfo(path, file_obj=None):
"""Returns a tuple of size infos (before, after). """Returns a tuple of size infos (before, after).
To reconstruct the DeltaSizeInfo, diff the two size infos. To reconstruct the DeltaSizeInfo, diff the two size infos.
""" """
with file_obj or open(filename, 'rb') as f: if not file_obj:
combined_header = _COMMON_HEADER + _SIZEDIFF_HEADER with open(path, 'rb') as f:
actual_header = f.read(len(combined_header)) return LoadDeltaSizeInfo(path, f)
if actual_header != combined_header:
raise Exception('Bad file header.') combined_header = _COMMON_HEADER + _SIZEDIFF_HEADER
actual_header = file_obj.read(len(combined_header))
json_len = int(f.readline()) if actual_header != combined_header:
json_str = f.read(json_len + 1) # + 1 for \n raise Exception('Bad file header.')
fields = json.loads(json_str)
json_len = int(file_obj.readline())
assert fields['version'] == _SIZEDIFF_VERSION json_str = file_obj.read(json_len + 1) # + 1 for \n
after_pos = f.tell() + fields['before_length'] fields = json.loads(json_str)
before_size_info = LoadSizeInfo(filename, f) assert fields['version'] == _SIZEDIFF_VERSION
f.seek(after_pos) after_pos = file_obj.tell() + fields['before_length']
after_size_info = LoadSizeInfo(filename, f)
before_size_info = LoadSizeInfo(path, file_obj)
file_obj.seek(after_pos)
after_size_info = LoadSizeInfo(path, file_obj)
return before_size_info, after_size_info return before_size_info, after_size_info
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