Add GetResponses() and related funcionality

tools/chrome_proxy/webdriver/common.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 argparse
 import json
 import os
+import re
 import shlex
 import sys
 import time
 import traceback
 
 sys.path.append(os.path.join(os.path.dirname(__file__), os.pardir, os.pardir,
   os.pardir, 'third_party', 'webdriver', 'pylib'))
 from selenium import webdriver
 from selenium.webdriver.chrome.options import Options
 
 # TODO(robertogden) add logging
 
 
 def ParseFlags():
   """
   Parses the given command line arguments and returns an object with the flags
   as properties.
   """
   parser = argparse.ArgumentParser()
   parser.add_argument('--browser_args', nargs=1, type=str, help='Override '
     'browser flags in code with these flags')
   parser.add_argument('--via_header_value', metavar='via_header', nargs=1,
     default='1.1 Chrome-Compression-Proxy', help='What the via should match to '
     'be considered valid')
   parser.add_argument('--chrome_exec', nargs=1, type=str, help='The path to '
     'the Chrome or Chromium executable')
   parser.add_argument('chrome_driver', nargs=1, type=str, help='The path to '
     'the ChromeDriver executable. If not given, the default system chrome '
     'will be used.')
-  # TODO(robertogden) make this a logging statement
-  print 'DEBUG: Args=', json.dumps(vars(parser.parse_args(sys.argv[1:])))
+  # TODO(robertogden) make a logging statement here

[sclittle, 2016/12/06 00:09:49]

style nit: You should have a colon after the "TODO(robertogden)" and all
comments should be complete sentences.

Also, could you clarify what you mean by "make a logging statement here" in the
comment?

[Robert Ogden, 2016/12/06 16:57:16]

Done.

   return parser.parse_args(sys.argv[1:])
 
 def HandleException(test_name=None):
   """
   Writes the exception being handled and a stack trace to stderr.
   """
   sys.stderr.write("**************************************\n")
   sys.stderr.write("**************************************\n")
   sys.stderr.write("**                                  **\n")
   sys.stderr.write("**       UNCAUGHT EXCEPTION         **\n")
@@ skipping 135 matching lines @@
   # TODO(robertogden) add timeout
   def ExecuteJavascript(self, script):
     """
     Executes the given javascript in the browser's current page as if it were on
     the console. Returns a string of whatever the evaluation was.
     """
     if not self._driver:
       self._StartDriver()
     return self._driver.execute_script("return " + script)
 
+  def GetPerformanceLogs(self, method_filter=r'Network\.responseReceived'):

[sclittle, 2016/12/06 00:09:49]

For all of these class and function definitions, you should be following the
Google Python style guide, e.g. starting with a single summary line.

See the style guide for more details:
https://google.github.io/styleguide/pyguide.html?showone=Comments#Comments

You can fix the style of the existing code here in a separate CL if you'd like,
but eventually it should follow the style guide.

[Robert Ogden, 2016/12/06 16:57:16]

Done.

+    """
+    Returns all logged Performance events from Chrome as a list of dicts, since
+    the last time this function was called. method_filter accepts a regex to
+    match against the method type of the events to return.
+    """
+    all_messages = []
+    for log in self._driver.execute('getLog', {'type': 'performance'})['value']:
+      message = json.loads(log['message'])['message']
+      if re.match(method_filter, message['method']):
+        all_messages.append(message)
+    return all_messages

[sclittle, 2016/12/06 00:09:49]

nit: This is minor, but if you don't need the entire list of logs at once, you
might consider making this function a generator (e.g. using "yield" instead of
"return"), so that each yielded value will be iterated over.

Same with the GetHTTPResponses() below.

[Robert Ogden, 2016/12/06 16:57:16]

Thought about that too. Kept it like this so that the len() function would still
work, which seems like it could be useful with all the counting and checking we
did in Telemetry.

+
+  def GetHTTPResponses(self, include_favicon=False):
+    """
+    Parses the Performance Logs and returns a list of HTTPResponse objects, each
+    representing a single completed HTTP transaction by Chrome. Optionally
+    allows favicon requests to be included in the result as well.
+    This function should be called exactly once after every page load.
+    """
+    def MakeHTTPResponse(log_dict):
+      params = log_dict['params']
+      response_dict = params['response']
+      response_headers = response_dict['headers']
+      request_headers = response_dict['requestHeaders']
+      url = response_dict['url']
+      protocol = request_headers[':scheme']
+      status = response_dict['status']
+      request_type = params['type']
+      return HTTPResponse(response_headers, request_headers, url, protocol,

[sclittle, 2016/12/06 00:09:49]

nit: Instead of making so many local variables, could you just pass in the dict
lookups directly to the constructor here?

e.g. return HTTPResponse(response_dict['headers'], ...)

[Robert Ogden, 2016/12/06 16:57:16]

Done.

+        status, request_type)
+    all_responses = []
+    for message in self.GetPerformanceLogs():
+      response = MakeHTTPResponse(message)
+      is_favicon = response.url.endswith('favicon.ico')
+      if not is_favicon or include_favicon:
+        all_responses.append(response)
+    return all_responses
+
+class HTTPResponse:
+  """
+  This class represents a single HTTP transaction (request and response) by
+  Chrome. This class also includes several convenience functions for ChromeProxy
+  specific assertions.
+  """
+
+  def __init__(self, response_headers, request_headers, url, protocol,
+      status, request_type):
+    self._response_headers = response_headers
+    self._request_headers = request_headers
+    self._url = url
+    self._protocol = protocol
+    self._status = status
+    self._request_type = request_type
+    self._flags = ParseFlags()
+
+  def __str__(self):
+    self_dict = {
+      'response_headers': self._response_headers,
+      'request_headers': self._request_headers,
+      'url': self._url,
+      'protocol': self._protocol,
+      'status': self._status,
+      'request_type': self._request_type
+    }
+    return json.dumps(self_dict)
+
+  @property
+  def response_headers(self):
+    return self._response_headers
+
+  @property
+  def request_headers(self):
+    return self._request_headers
+
+  @property
+  def url(self):
+    return self._url
+
+  @property
+  def protocol(self):
+    return self._protocol
+
+  @property
+  def status(self):
+    return self._status
+
+  @property
+  def request_type(self):
+    return self._request_type
+
+  def UsedHTTPS(self):
+    return self._protocol == 'https'

[sclittle, 2016/12/06 00:09:49]

What do you intent |UsedHTTPS| to be used for? I haven't confirmed this, but I'd
guess that it checks if the fetched URL used HTTPS, regardless of whether or not
the secure proxy was used.

IIRC we have a of couple tests (e.g. HTTPFallbackToDirect or something) that
check that the secure or insecure proxy is specifically being used.

If those are what you want to use |UsedHTTPS| for, then could you confirm that
it does what you want it to do?

[Robert Ogden, 2016/12/06 16:57:15]

Correct, there are a number of tests where we need to check for HTTPS vs HTTP.
and yes, this is working as intended.

+
+  def ResponseHasViaHeader(self):
+    return 'via' in self._response_headers and (self._response_headers['via'] ==
+      self._flags.via_header_value)
+
+  def WasXHR(self):
+    return self.request_type == 'XHR'
 
 class IntegrationTest:
   """
   A parent class for all integration tests with utility and assertion methods.
   All methods starting with the word 'test' (ignoring case) will be called with
   the RunAllTests() method which can be used in place of a main method.
   """
   def RunAllTests(self):
     """
     Runs all methods starting with the word 'test' (ignoring case) in the class.
@@ skipping 13 matching lines @@
   def Fail(self, msg):
     sys.stderr.write("**************************************\n")
     sys.stderr.write("**************************************\n")
     sys.stderr.write("**                                  **\n")
     sys.stderr.write("**          TEST FAILURE            **\n")
     sys.stderr.write("**                                  **\n")
     sys.stderr.write("**************************************\n")
     sys.stderr.write("**************************************\n")
     sys.stderr.write(msg, '\n')
     sys.exit(1)