Changeset 89808 in webkit

Timestamp:

Jun 27, 2011 1:41:55 AM (13 years ago)

Author:

eric@webkit.org

Message:

2011-06-27 Eric Seidel <​eric@webkit.org>

Reviewed by Adam Barth.

Remove evil uses of hasattr
​https://bugs.webkit.org/show_bug.cgi?id=63430

For some reason these classes believe that they may be called with
various flavors of "option" elements and so carefully check to make
sure that the options element has their option before checking it.

We had a set_option_default method which was never called, so I made it
do what callsites seemed to want it to do and replaced 3 callers
who previously used hasattr manually to use set_option_default instead.

• Scripts/webkitpy/layout_tests/port/base.py:
• Scripts/webkitpy/layout_tests/port/webkit.py:

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/layout_tests/port/base.py (modified) (2 diffs)
• Scripts/webkitpy/layout_tests/port/webkit.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2011-06-27  Eric Seidel  <eric@webkit.org>
+
+        Reviewed by Adam Barth.
+
+        Remove evil uses of hasattr
+        https://bugs.webkit.org/show_bug.cgi?id=63430
+
+        For some reason these classes believe that they may be called with
+        various flavors of "option" elements and so carefully check to make
+        sure that the options element has their option before checking it.
+
+        We had a set_option_default method which was never called, so I made it
+        do what callsites seemed to want it to do and replaced 3 callers
+        who previously used hasattr manually to use set_option_default instead.
+
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/layout_tests/port/webkit.py:
+
 2011-06-27  Kent Tamura  <tkent@chromium.org>
 

trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py

@@ -137 +137 @@
         self._pretty_patch_available = None
 
-        if not hasattr(self._options, 'configuration') or self._options.configuration is None:
-            self._options.configuration = self.default_configuration()
+        self.set_option_default('configuration', self.default_configuration())
         self._test_configuration = None
         self._multiprocessing_is_available = (multiprocessing is not None)
         self._results_directory = None
-
-        if not hasattr(self._options, 'use_apache') or self._options.use_apache is None:
-            self._options.use_apache = self._default_to_apache()
+        self.set_option_default('use_apache', self._default_to_apache())
 
     def wdiff_available(self):
@@ -583 +580 @@
 
     def set_option_default(self, name, default_value):
-        if not hasattr(self._options, name):
+        # FIXME: Callers could also use optparse_parser.Values.ensure_value,
+        # since this should always be a optparse_parser.Values object.
+        if not hasattr(self._options, name) or getattr(self._options, name) is None:
             return setattr(self._options, name, default_value)
 

trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py

@@ -56 +56 @@
         self._cached_apache_path = None
 
-        # FIXME: disable pixel tests until they are run by default on the
-        # build machines.
-        if not hasattr(self._options, "pixel_tests") or self._options.pixel_tests == None:
-            self._options.pixel_tests = False
+        # FIXME: disable pixel tests until they are run by default on the build machines.
+        self.set_option_default("pixel_tests", False)
 
     def baseline_search_path(self):