Fix another source of utf8 characters being lost.

appengine/chromium_build/app.py

 # Copyright (c) 2012 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.
 
 from __future__ import with_statement
 
 import datetime
 import json
 import logging
 import os
@@ skipping 257 matching lines @@
         'comment': None,
         'details': None,
     }
     prev_rev_db = get_or_create_row('latest_rev', rev_number)
     prev_rev_db.fetch_timestamp = datetime.datetime.now()
     prev_rev_db.rev_number = rev_number
     prev_rev_db.put()
     put_data_into_cache('latest_rev', latest_rev_row)
 
 
+def utf8_convert(beautiful_soup_tag):
+  # cmp also investigated:
+  #   beautiful_soup_tag.__str__(encoding='utf-8').decode('utf-8')
+  # He found that the BeautifulSoup() __str__ method when used with a 'utf-8'
+  # encoding returned effectively the same thing as str(), a Python built-in.
+  # After a handful of tests, he switched to using str() to avoid the add'l
+  # complexity of another BeautifulSoup method.
+  return str(beautiful_soup_tag).decode('utf-8')
+
+
 ##########
 # ConsoleData class definition and related functions.
 ##########
 class ConsoleData(object):
   def __init__(self):
     self.row_orderedkeys = []
     self.row_data = {}
 
     # Retain order of observed masters.
     self.masters = []
 
     # Map(k,v): k=Master, v=List of categories
     self.category_order = {}
     # Map(k,v): k=Master, v=Dict of category data
     self.category_data = {}
 
     self.category_count = 0
     self.master = ''
     self.lastRevisionSeen = None
     self.lastMasterSeen = None
 
   @staticmethod
   def ContentsToHtml(contents):
-    return ''.join(unicode(content).encode('ascii', 'replace')
-                   for content in contents)

[cmp, 2015/02/12 18:37:16]

This caused the UTF-8 characters |content| was holding to be downconverted into
?s...

+    return ''.join(utf8_convert(content) for content in contents)
 
   @property
   def last_row(self):
     return self.row_data[self.lastRevisionSeen]
 
   def SawMaster(self, master):
     self.lastMasterSeen = master
     assert(self.lastMasterSeen not in self.category_order)
     self.masters.append(self.lastMasterSeen)
     self.category_order.setdefault(self.lastMasterSeen, [])
@@ skipping 81 matching lines @@
 
 
 ##########
 # Heavy-lifting functions that do most of the console processing.
 # AKA postfetch and postsave functions/handlers.
 ##########
 def console_merger(localpath, remoteurl, page_data,
                    masters_to_merge=None, num_rows_to_merge=None):
   masters_to_merge = masters_to_merge or DEFAULT_MASTERS_TO_MERGE
   num_rows_to_merge = num_rows_to_merge or 25
-  mergedconsole = ConsoleData()
+  console_data = ConsoleData()
   surroundings = get_and_cache_pagedata('surroundings')
   merged_page = BeautifulSoup(surroundings['content'])
   merged_tag = merged_page.find('table', 'ConsoleData')
   if merged_tag is None:
     msg = 'console_merger("%s", "%s", "%s"): merged_tag cannot be None.' % (
           localpath, remoteurl, page_data)
     logging.error(msg)
     raise Exception(msg)
   latest_rev = int(get_and_cache_rowdata('latest_rev')['rev_number'])
   if not latest_rev:
     logging.error('console_merger(\'%s\', \'%s\', \'%s\'): cannot get latest '
                   'revision number.' % (
                       localpath, remoteurl, page_data))
     return
   fetch_timestamp = datetime.datetime.now()
   for master in masters_to_merge:
     # Fetch the summary one-box-per-builder for the master.
     # If we don't get it, something is wrong, skip the master entirely.
     master_summary = get_and_cache_pagedata('%s/console/summary' % master)
     if not master_summary['content']:
       continue
-    mergedconsole.SawMaster(master)
+    console_data.SawMaster(master)
     # Get the categories for this builder. If the builder doesn't have any
     # categories, just use the default empty-string category.
     category_list = []
     master_categories = get_and_cache_pagedata('%s/console/categories' % master)
     if not master_categories['content']:
       category_list.append('')
     else:
       category_row = BeautifulSoup(master_categories['content'])
       category_list = [c.text for c in category_row.findAll('td', 'DevStatus')]
     # Get the corresponding summary box(es).
     summary_row = BeautifulSoup(master_summary['content'])
     summary_list = summary_row.findAll('table')
     for category, summary in zip(category_list, summary_list):
-      mergedconsole.AddCategory(category, summary)
+      console_data.AddCategory(category, summary)
 
     # Fetch all of the rows that we need.
     rows_fetched = 0
     revs_skipped = 0
     current_rev = latest_rev
     while rows_fetched < num_rows_to_merge and current_rev >= 0:
       # Don't get stuck looping backwards forever into data we don't have.
       # How hard we try scales with how many rows the person wants.
       if revs_skipped > max(num_rows_to_merge, 10):
         break
       row_data = get_and_cache_rowdata('%s/console/%s' % (master, current_rev))
       if not row_data:
         current_rev -= 1
         revs_skipped += 1
         continue
-      mergedconsole.AddRow(row_data)
+      console_data.AddRow(row_data)
       current_rev -= 1
       revs_skipped = 0
       rows_fetched += 1
 
   # Convert the merged content into console content.
-  mergedconsole.Finish()
+  console_data.Finish()
   template_environment = Environment()
   template_environment.loader = FileSystemLoader('.')
   def notstarted(builder_status):
     """Convert a BeautifulSoup Tag from builder status to a notstarted line."""
     builder_status = re.sub(r'DevSlaveBox', 'DevStatusBox', str(builder_status))
     builder_status = re.sub(r'class=\'([^\']*)\' target=',
                             'class=\'DevStatusBox notstarted\' target=',
                             builder_status)
     builder_status = re.sub(r'class="([^"]*)" target=',
                             'class="DevStatusBox notstarted" target=',
                             builder_status)
     return builder_status
   template_environment.filters['notstarted'] = notstarted
   merged_template = template_environment.from_string(console_template)
-  merged_content = merged_template.render(data=mergedconsole)
+  merged_console = merged_template.render(data=console_data)
   # For debugging:
-  # print merged_content
+  # logging.info('%r' % merged_console)
   # import code
   # code.interact(local=locals())
 
-  # Place merged data at |merged_tag|'s location in |merged_page|, and put the
-  # result in |merged_content|.
-  merged_tag.replaceWith(str(merged_content))

[cmp, 2015/02/12 18:37:16]

And once ContentsToHtml wasn't downsampling to ?, this line would crash. 
.render() is a jinja method that returns a Unicode string.  Any attempt to run
str() on that string when it held Unicode characters would trigger the crash.

-  # .prettify() may damage the HTML but makes output nicer.  However, that
-  # cost is a bunch of extra whitespace.  We reduce page size by not using
-  # .prettify().
-  merged_content = merged_page.__str__(encoding=None)

[cmp, 2015/02/12 18:37:16]

This line was causing the DTD doubling in this test, test_console_merger, and
test_console_merger_splitrevs.  merged_page is a BeautifulSoup object and so if
one wants a Unicode string it must be handled specifically to convert to a
non-Unicode string, and then decode it back into Unicode.

+  # Place merged console at |merged_tag|'s location in |merged_page|, and put
+  # the result in |merged_content|.
+  merged_tag.replaceWith(merged_console)
+  merged_content = utf8_convert(merged_page)
   merged_content = re.sub(
       r'\'\<a href="\'', '\'<a \' + attributes + \' href="\'', merged_content)
   merged_content = re.sub(
       r'\'\<table\>\'', r"'<table ' + attributes + '>'", merged_content)
   merged_content = re.sub(
       r'\'\<div\>\'', r"'<div ' + attributes + '>'", merged_content)
   merged_content = re.sub(
       r'\'\<td\>\'', r"'<td ' + attributes + '>'", merged_content)
   merged_content = re.sub(
       r'\<iframe\>\</iframe\>',
@@ skipping 163 matching lines @@
   # exist more than once.  Reverts are examples of commits which can contain
   # multiple Cr-Commit-Position instances.  In those cases, only the last one
   # is correct, so split on the break tag and reverse the result to find the
   # last occurrence of Cr-Commit-Position.
   for line in reversed(commit_msg.split('<br />')):
     if line.startswith('Cr-Commit-Position: '):
       return filter(str.isdigit, str(line.split('@')[-1]))
   return '0'
 
 
-def utf8_convert(bstring):
-  # cmp also investigated:
-  #   bstring.__str__(encoding='utf-8').decode('utf-8')
-  # He found that the BeautifulSoup() __str__ method when used with a 'utf-8'
-  # encoding returned effectively the same thing as str(), a Python built-in.
-  # After a handful of tests, he switched to using str() to avoid the add'l
-  # complexity of another BeautifulSoup method.
-  return str(bstring).decode('utf-8')
-
-
 # W0613:600,28:parse_master: Unused argument 'remoteurl'
 # pylint: disable=W0613
 def parse_master(localpath, remoteurl, page_data=None):
   """Part of the new pipeline to store individual rows rather than
   whole pages of html. Parses the master data into a set of rows,
   and writes them out to the datastore in an easily retrievable format.
 
   Doesn't modify page_data dict.
   """
   ts = datetime.datetime.now()
@@ skipping 558 matching lines @@
          'builds/-1?as_text=1'),
     'localpath':
         'chromium.lkgr/json/builders/Linux%20x64/builds/-1/as_text=1.json',
     'maxage': 2*60,  # 2 mins
   },
 
 # # Trigger background process update.
 # {
 #     'remoteurl': 'http://chromium-build.appspot.com/backend/update'
 ]