[Findit] Factoring out the components, so they can classify themselves

appengine/findit/crash/component.py

+# Copyright 2016 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.
+
+import collections
+import re
+
+
+class Component(collections.namedtuple('Component',
+    ['component_name', 'path_regex', 'function_regex'])):
+  """A representation of a "component" in Chromium.
+
+  For example: 'Blink>DOM' or 'Blink>HTML'. Notably, a component knows
+  how to identify itself. Hence, given a stack frame or change list
+  or whatever, we ask the Component whether it matches that frame,
+  CL, etc."""
+  __slots__ = ()
+
+  def __new__(cls, component_name, path_regex, function_regex=None):
+    return super(cls, Component).__new__(cls,
+      component_name,
+      re.compile(path_regex),

[Sharu Jiang, 2016/09/20 00:31:29]

We don't need to compile the regex here, since they are already compiled in
Config.

[wrengr, 2016/09/27 22:00:39]

My aim is to remove the config's compilation, since that's not the right place
to do it. However, when I tried removing it there some strange things started
breaking with the way mocking is done. I fixed what I could there, but couldn't
fix everything. Calling re.compile on an already compiled regex is a no-op, so
it's perfectly fine to do this until the config code gets fixed.

[Sharu Jiang, 2016/09/27 22:31:03]

The reason we do the compilation in config is that the classifier only needs to
be compiled once there is update in config. If we move this to Component, we
need to compile the regex for every crash.

[wrengr, 2016/09/27 22:54:32]

Conceptually, Component is the right place to do the compilation. This is
because the Component is what conceptually "owns" the regex; the config file is
just a way for humans to specify which Component objects should be built. If
there's a worry about the cost of building multiple copies of a Component, then
the Component objects should be saved somewhere and reused.

Conceptually, the right place to save Component objects would be in whatever
object "owns" them, namely some object encapsulating the whole idea of analyzing
crash reports. Right now there is no such class, but basically it'd provide a
home for the stuff currently living in places like findit_for_chromecrash.py.
The current FindCulpritForChromeCrash function would become a FindCulprit method
on the Chrome Crash instance of the putative class; which would address your
similar concern raised about that function in a different CL.

+      re.compile(function_regex) if function_regex else None)
+
+
+  def MatchesStackFrame(self, frame):
+    """Does this component match the file path and function of the frame?"""

[stgao, 2016/09/21 21:57:27]

nit: use a statement instead of a question. (I understand this function is to
answer the question, but this is not the convention in the infra repo).

[wrengr, 2016/09/27 22:00:39]

Done.

+    if not self.path_regex.match(frame.dep_path + frame.file_path):
+      return False
+
+    # We interpret function_regex=None to mean the regex that matches everything

[Martin Barbella, 2016/09/19 21:17:18]

Nit: end with a period.

[wrengr, 2016/09/27 22:00:39]

Done.

+    if not self.function_regex:
+      return True
+    return self.function_regex.match(frame.function)
+