[Findit] Plain objects to represent and parse stack trace.

tools/findit/component_dictionary.py

Index: tools/findit/component_dictionary.py
diff --git a/tools/findit/component_dictionary.py b/tools/findit/component_dictionary.py
new file mode 100644
index 0000000000000000000000000000000000000000..e5821dfce1e686c642df47a399cf433e4c8aae85
--- /dev/null
+++ b/tools/findit/component_dictionary.py
@@ -0,0 +1,114 @@
+# Copyright (c) 2014 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.
+
+
+class ComponentDictionary(object):
+  """Represents a file dictionary.
+
+  It maps each component (blink, chrome, etc) to a component dictionary.

[stgao, 2014/07/31 01:52:24]

component dictionary? or should be file dictionary?

[jeun, 2014/07/31 18:41:14]

Done.

+  """
+
+  def __init__(self, components):
+    """Initializes the dictionary with given components."""
+    self.component_dic = {}
+    self.components = components

[stgao, 2014/07/31 01:52:23]

Do we need to save |components| here?

It could be derived from component_dic.keys()

[jeun, 2014/07/31 18:41:14]

Done.

+
+    # Create file dictionary for all the components
+    for component in components:
+      self.component_dic[component] = FileDictionary()

[stgao, 2014/07/31 01:52:23]

As FileDictionary is used by ComponentDictionary, FileDictionary should go
first.

Same for classes in stacktrace.py

[jeun, 2014/07/31 18:41:14]

Done.

+
+  def __iter__(self):
+    return iter(self.component_dic)
+
+  def GetFileDic(self, component):
+    """Returns a component dictionary for a given component."""

[stgao, 2014/07/31 01:52:24]

FileDictionary or component dictionary?

[jeun, 2014/07/31 18:41:14]

Done.

+    return self.component_dic[component]
+
+  def FromStacktrace(self, stack_list):

[stgao, 2014/07/31 01:52:24]

|stack_list| is misleading here.
Should it be |stack_frame_list| or |stack_frames|?

[jeun, 2014/07/31 18:41:14]

Done.

+    """Parses stacktrace, given an instance of StacktraceLine list."""
+    # Iterate through the list of stacktrace line objects
+    for stacktrace_line in stack_list:
+

[stgao, 2014/07/31 01:52:23]

This empty line could be removed.

[jeun, 2014/07/31 18:41:14]

Done.

+      # If the component of this line is not believed to have caused the
+      # crash, ignore this line

[stgao, 2014/07/31 01:52:24]

How do you know this here?

[jeun, 2014/07/31 18:41:14]

Updated the comment to explain this in a better way

+      component = stacktrace_line.component
+      if component not in self.components:

[stgao, 2014/07/31 01:52:24]

we could do below instead:

if component not in self.component_dic:

[jeun, 2014/07/31 18:41:14]

Done.

+        continue
+
+      # Get values of the variables
+      file_name = stacktrace_line.file_name
+      file_path = stacktrace_line.file_path
+      crashed_line = stacktrace_line.crashed_line
+      stack_frame_index = stacktrace_line.stack_frame_index
+      function = stacktrace_line.function
+
+      # Add the file to this component's dictionary of files
+      file_dic = self.component_dic[component]
+      file_dic.AddFile(file_name, file_path, crashed_line, stack_frame_index,
+                       function)
+
+
+class FileDictionary(object):
+  """Maps file in a stacktrace to its crash information.
+
+  It maps file to another dictionary, which maps the file's path to crashed
+  lines, stack frame indices and crashed functions.
+  """
+
+  def __init__(self):
+    """Initializes the component dictionary."""
+    self.file_dic = {}
+
+  def AddFile(self, file_name, file_path, crashed_line,
+              stack_frame_index, function):
+    """Adds file and its crash information to the map.
+
+    Args:
+      file_name: name of the crashed file

[stgao, 2014/07/31 01:52:23]

I think file_name is not needed, as it could be derived from file_path by
os.path.basename(file_path)

[jeun, 2014/07/31 18:41:14]

I think because stacktrace line already has file_name as one of its fields, it
might be OK to just pass it in instead of recalculating it. If we want to remove
file name overall, I'm not sure if it is better to calculate every time or to
just pass it as an argument.

[stgao, 2014/08/01 17:22:20]

OK, that's fine. It is a problem of time and memory.

+      file_path: path of the crashed file
+      crashed_line: crashed line of the file
+      stack_frame_index: the file's position in the stack frame
+      function: name of the crashed function
+    """
+

[stgao, 2014/07/31 01:52:23]

Empty line is not needed.

[jeun, 2014/07/31 18:41:14]

Done.

+    # Populate the dictionary if this file/path has not been added before
+    if file_name not in self.file_dic:
+      self.file_dic[file_name] = {}
+
+    if file_path not in self.file_dic[file_name]:
+      self.file_dic[file_name][file_path] = {}
+
+    if 'lines' not in self.file_dic[file_name][file_path]:
+      self.file_dic[file_name][file_path]['lines'] = []
+
+    if 'stack_frame_index' not in self.file_dic[file_name][file_path]:
+      self.file_dic[file_name][file_path]['stack_frame_index'] = []
+
+    if 'function' not in self.file_dic[file_name][file_path]:
+      self.file_dic[file_name][file_path]['function'] = []
+
+    # Add the crashed line, frame index and function name
+    self.file_dic[file_name][file_path]['lines'].append(crashed_line)
+    self.file_dic[file_name][file_path]['stack_frame_index'].append(
+        stack_frame_index)
+    self.file_dic[file_name][file_path]['function'].append(function)
+
+  def GetPathDic(self, file_name):

[stgao, 2014/07/31 01:52:23]

For these Get functions here, could we just use file_path? Because file_name is
derivable from it.

[jeun, 2014/07/31 18:41:14]

Done.

+    """Returns file's path and crash information."""
+    return self.file_dic[file_name]
+
+  def GetCrashedLines(self, file_name, file_path):
+    """Returns crashed lines given a file name and path."""
+    return self.file_dic[file_name][file_path]['lines']
+
+  def GetCrashStackFrameindex(self, file_name, file_path):
+    """Returns stack frame indices given a file name and path."""
+    return self.file_dic[file_name][file_path]['stack_frame_index']
+
+  def GetCrashFunction(self, file_name, file_path):
+    """Returns list of crashed functions given a file name and path."""
+    return self.file_dic[file_name][file_path]['function']
+
+  def __iter__(self):
+    return iter(self.file_dic)