bisect-builds.py: Minor bug fixes for blink bisect and better error handling.

tools/bisect-builds.py

Index: tools/bisect-builds.py
diff --git a/tools/bisect-builds.py b/tools/bisect-builds.py
index fb4bac250d544b5ee793f14c4e85d3a97c2dbe9d..7d2156e7f9c466b5da44af3a17689f09ca7921b5 100755
--- a/tools/bisect-builds.py
+++ b/tools/bisect-builds.py
@@ -321,7 +321,12 @@ class PathContext(object):
     except urllib.HTTPError as error:
       msg = 'HTTP Error %d for %s' % (error.getcode(), git_sha1)
       return None
-    data = json.loads(response.read()[4:])
+    try:
+      response_data = response.read()
+      data = json.loads(response_data[4:])
+    except ValueError:
+      print 'Response string for %s is "%s"' % (json_url, response_data)
+      raise ValueError
     if 'message' in data:
       message = data['message'].split('\n')
       message = [line for line in message if line.strip()]
@@ -842,7 +847,7 @@ def Bisect(base_url,
 
     rev = revlist[pivot]
 
-  return (revlist[minrev], revlist[maxrev])
+  return (revlist[minrev], revlist[maxrev], context)
 
 
 def GetBlinkDEPSRevisionForChromiumRevision(rev):
@@ -872,12 +877,17 @@ def GetBlinkRevisionForChromiumRevision(self, rev):
   file_url = '%s/%s%s/REVISIONS' % (self.base_url,
                                     self._listing_platform_dir, rev)
   url = urllib.urlopen(file_url)
-  data = json.loads(url.read())
+  try:
+    response = url.read()
+    data = json.loads(response)
+  except ValueError:
+    print 'Response from %s: "%s"' % (file_url, response)
+    raise ValueError
   url.close()
   if 'webkit_revision' in data:
     blink_rev = data['webkit_revision']
     if not _IsRevisionNumber(blink_rev):
-      blink_rev = self.GetSVNRevisionFromGitHash(blink_rev, 'blink')
+      blink_rev = int(self.GetSVNRevisionFromGitHash(blink_rev, 'blink'))
     return blink_rev
   else:
     raise Exception('Could not get blink revision for cr rev %d' % rev)
@@ -1089,7 +1099,7 @@ def main():
   else:
     evaluator = AskIsGoodBuild
 
-  (min_chromium_rev, max_chromium_rev) = Bisect(
+  (min_chromium_rev, max_chromium_rev, context) = Bisect(

[Robert Sesek, 2014/08/12 20:50:11]

Why do you need to return context if you're not using it?

[pshenoy, 2014/08/12 21:05:06]

'context' is passed in GetBlinkRevisionForChromiumRevision to access
githash_svn_dict. So we need the 'context' created in Bisect() to get the blink
revision from the corresponding githash value. Earlier it was using the
'context' created in main which was dummy.

[Robert Sesek, 2014/08/12 21:41:24]

Ah, makes sense. Why don't we switch from passing all these parameters to
Bisect, which merely passes them to the PathContext constructor, to creating the
context here in Main and then just passing the context to Bisect.

[pshenoy, 2014/08/13 18:13:41]

Done.

       base_url, opts.archive, opts.official_builds, opts.aura, opts.asan,
       opts.use_local_repo, good_rev, bad_rev, opts.times, opts.command,
       args, opts.profile, opts.flash_path, opts.pdf_path,