Add baked-in favicons for default popular sites on NTP

components/ntp_tiles/update_default_sites_resources.py

+#!/usr/bin/env python

[sfiera, 2017/02/13 18:36:35]

Is this file written according to
http://google.github.io/styleguide/pyguide.html?

It doesn't seem to, w.r.t. imports, naming, and main().

[fhorschig, 2017/02/14 17:29:02]

Thanks for the link!
It was not (minimal linting/presubmit requirements).
All violations I found are gone but I consider looking
for someone who wrote Chromium python code before...

+# Copyright 2017 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 sys, requests, os, json, urllib, re
+
+kDefaultPopularSites =
+    'https://www.gstatic.com/chrome/ntp/suggested_sites_DEFAULT_5.json'
+kSiteIconName = 'icon'
+kNtpTilesResourcePath = os.path.join(
+    os.path.dirname(os.path.realpath(__file__)), 'resources')
+kDefaultPopularSitesPath = os.path.join(kNtpTilesResourcePath,
+                                        'default_popular_sites.json')
+
+print("Downloading default popular sites...  (" + kDefaultPopularSites + ")")
+data = requests.get(url=kDefaultPopularSites).json()
+print("... done. (" + str(len(data)) + " sites found)")

[sfiera, 2017/02/13 18:36:35]

print("... done. (%d sites found)" % len(data))

[fhorschig, 2017/02/15 18:14:34]

Done.

+
+with open(kDefaultPopularSitesPath, 'w') as f:
+  json.dump(data, f)
+print("JSON was written to " + kDefaultPopularSitesPath)
+
+"""Deleting old icons explicitly avoids wrong icons for pages without such."""

[sfiera, 2017/02/13 18:36:35]

Regular comments are #. Only docstrings have """

[fhorschig, 2017/02/15 18:14:34]

Done.

+print("Deleting old icons..")
+for f in os.listdir(kNtpTilesResourcePath):
+  if re.search(kSiteIconName + "*", f):

[sfiera, 2017/02/13 18:36:35]

I think you are looking for https://docs.python.org/2/library/glob.html

(Also, doesn't this regexp match ico, icon, iconnnnn, etc.?)

[fhorschig, 2017/02/15 18:14:35]

nice!
(Yes, it was intentional but not entirely correct. See new regexp)

+      os.remove(os.path.join(kNtpTilesResourcePath, f))
+print("... done.")
+
+for i in range(len(data)):
+  print("Downloading icon for \"" + data[i]['title']) +"\"..."
+  print("(Source: " + data[i]['large_icon_url']) +")"

[sfiera, 2017/02/13 18:36:35]

Not all sites have large_icon_url. It's probably safe to assume that all sites
on the default list have it, but I'd leave a comment here for the benefit of the
hapless user that might one day see a KeyError here.

[fhorschig, 2017/02/15 18:14:34]

Done.

+  urllib.urlretrieve(data[i]['large_icon_url'],

[sfiera, 2017/02/13 18:36:35]

Why are you using requests above and urllib here?

[fhorschig, 2017/02/14 17:29:02]

Requests is an easy way to get the response as JSON whereas
just downloading a file is easier with urllib.
I don't see a good reason to use just one library (although
possible for either) but if you name one, I will change it.

[fhorschig, 2017/02/15 18:14:34]

I used urllib everywhere. requests ran into some problem with
https pages.

+                     os.path.join(kNtpTilesResourcePath,
+                                  kSiteIconName + str(i)))
+  print("... done. (Stored as " + kSiteIconName + str(i) + ")");