Changeset 249095 in webkit

Timestamp:

Aug 26, 2019, 7:36:47 AM (7 years ago)

Author:

clopez@igalia.com

Message:

Missing media controls when WebKit is built with Python3
​https://bugs.webkit.org/show_bug.cgi?id=194367

Reviewed by Carlos Garcia Campos.

The JavaScript minifier script jsmin.py expects a text stream
with text type as input, but the script make-js-file-arrays.py
was passing to it a FileIO() object. So, when the jsmin script
called read() over this object, python3 was returning a type of
bytes, but for python2 it returns type str.

This caused two problems: first that jsmin failed to do any minifying
because it was comparing strings with a variable of type bytes.
The second major problem was in the write() function, when the
jsmin script tried to convert a byte character to text by calling
str() on it. Because what this does is not to convert from byte
type to string, but to simply generate a string with the format b'c'.
So the jsmin script was returning back as minified JS complete
garbage in the form of "b't'b'h'b'h'b'i" for python3.

Therefore, when WebKit was built with python3 this broke everything
that depended on the embedded JS code that make-js-file-arrays.py
was supposed to generate, like the media controls and the WebDriver
atoms.

Fix this by reworking the code in make-js-file-arrays script to
read the data from the file using a TextIOWrapper in python 3
with decoding for 'utf-8'. This ensures that the jsmin receives
a text type. For python2 keep using the same FileIO class.

On the jsmin.py script remove the problematic call to str() inside
the write() function when running with python3.
On top of that, add an extra check in jsmin.py script to make it
fail if the character type read is not the one expected. This
will cause the build to fail instead of failing silently like
now. I did some tests and the runtime cost of this extra check
is almost zero.

• Scripts/jsmin.py:

(JavascriptMinify.minify.write):
(JavascriptMinify):

• Scripts/make-js-file-arrays.py:

(main):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• Scripts/jsmin.py (modified) (3 diffs)
• Scripts/make-js-file-arrays.py (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-08-26  Carlos Alberto Lopez Perez  <clopez@igalia.com>
+
+        Missing media controls when WebKit is built with Python3
+        https://bugs.webkit.org/show_bug.cgi?id=194367
+
+        Reviewed by Carlos Garcia Campos.
+
+        The JavaScript minifier script jsmin.py expects a text stream
+        with text type as input, but the script make-js-file-arrays.py
+        was passing to it a FileIO() object. So, when the jsmin script
+        called read() over this object, python3 was returning a type of
+        bytes, but for python2 it returns type str.
+
+        This caused two problems: first that jsmin failed to do any minifying
+        because it was comparing strings with a variable of type bytes.
+        The second major problem was in the write() function, when the
+        jsmin script tried to convert a byte character to text by calling
+        str() on it. Because what this does is not to convert from byte
+        type to string, but to simply generate a string with the format b'c'.
+        So the jsmin script was returning back as minified JS complete
+        garbage in the form of "b't'b'h'b'h'b'i" for python3.
+
+        Therefore, when WebKit was built with python3 this broke everything
+        that depended on the embedded JS code that make-js-file-arrays.py
+        was supposed to generate, like the media controls and the WebDriver
+        atoms.
+
+        Fix this by reworking the code in make-js-file-arrays script to
+        read the data from the file using a TextIOWrapper in python 3
+        with decoding for 'utf-8'. This ensures that the jsmin receives
+        a text type. For python2 keep using the same FileIO class.
+
+        On the jsmin.py script remove the problematic call to str() inside
+        the write() function when running with python3.
+        On top of that, add an extra check in jsmin.py script to make it
+        fail if the character type read is not the one expected. This
+        will cause the build to fail instead of failing silently like
+        now. I did some tests and the runtime cost of this extra check
+        is almost zero.
+
+        * Scripts/jsmin.py:
+        (JavascriptMinify.minify.write):
+        (JavascriptMinify):
+        * Scripts/make-js-file-arrays.py:
+        (main):
+
 2019-08-23  Devin Rousso  <drousso@apple.com>
 

trunk/Source/JavaScriptCore/Scripts/jsmin.py

@@ -29 +29 @@
 if is_3:
     import io
+    python_text_type = str
 else:
     import StringIO
@@ -35 +36 @@
     except ImportError:
         cStringIO = None
+    python_text_type = basestring
 
 
@@ -83 +85 @@
                 self.return_buf += char
                 self.is_return = self.return_buf == 'return'
-            if sys.version_info.major == 2:
-                self.outs.write(char)
-            else:
-                self.outs.write(str(char))
+            self.outs.write(char)
             if self.is_return:
                 self.return_buf = ''
 
-        read = self.ins.read
+        def read(n):
+            char = self.ins.read(n)
+            if not isinstance(char, python_text_type):
+                raise ValueError("ERROR: The script jsmin.py can only handle text input, but it received input of type %s" % type(char))
+            return char
 
         space_strings = "abcdefghijklmnopqrstuvwxyz"\

trunk/Source/JavaScriptCore/Scripts/make-js-file-arrays.py

@@ -27 +27 @@
 from optparse import OptionParser
 import sys
-if sys.version_info.major == 2:
-    from StringIO import StringIO
-else:
-    from io import StringIO
-from jsmin import JavascriptMinify
+from jsmin import jsmin
+is_3 = sys.version_info >= (3, 0)
 
 
@@ -72 +69 @@
     print('namespace {0:s} {{'.format(namespace), file=sourceFile)
 
-    jsm = JavascriptMinify()
+    for inputFileName in inputPaths:
 
-    for inputFileName in inputPaths:
-        inputStream = io.FileIO(inputFileName)
-        outputStream = StringIO()
+        if is_3:
+            inputStream = io.open(inputFileName, encoding='utf-8')
+        else:
+            inputStream = io.FileIO(inputFileName)
+
+        data = inputStream.read()
 
         if not options.no_minify:
-            jsm.minify(inputStream, outputStream)
-            characters = outputStream.getvalue()
+            characters = jsmin(data)
         else:
-            characters = inputStream.read()
+            characters = data
 
-        size = len(characters)
+        if is_3:
+            codepoints = bytearray(characters, encoding='utf-8')
+        else:
+            codepoints = list(map(ord, characters))
+
+        # Use the size of codepoints instead of the characters
+        # because UTF-8 characters may need more than one byte.
+        size = len(codepoints)
+
         variableName = os.path.splitext(os.path.basename(inputFileName))[0]
 
@@ -90 +97 @@
         print('const char {0:s}JavaScript[{1:d}] = {{'.format(variableName, size), file=sourceFile)
 
-        codepoints = list(map(ord, characters))
         for codepointChunk in chunk(codepoints, 16):
             print('    {0:s},'.format(','.join(map(stringifyCodepoint, codepointChunk))), file=sourceFile)