Docserver: Implement the content providers infrastructure, where a

Docserver: Implement the content providers infrastructure, where a
configuration file (content_providers.json) specifies arbitrary places to fetch
content from.

As a first pass only the Chromium repository is supported, and our existing 3
content providers (public, exampes, and static) have been moved over to use it.
In a follow-up GitHub will be supported as a content provider along with a
number of other features to content_providers.json so that we're able to host
the rest of Chrome's documentation, in theory.

BUG=293678
R=jyasskin@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=232868

[not at google - send to devlin, 2013-11-01 20:45:18]

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/server2/render_servlet.py (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/render_servlet.py:8: import posixpath
You may want to review this file as if it were newly added. It's changed very
dramatically, though it amounts to moving most of it into ContentProvider.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/templates/json/content_providers.json
(right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/templates/json/content_providers.json:6:
"supportsZip": true,
This does have the side-effect where any directory under extensions/examples can
be zipped up, not just the top-level ones. It doesn't seem like a big problem to
me.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/templates/public/redirects.json (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/templates/public/redirects.json:2: "":
"//developers.google.com/chrome",
Noticed this in testing.

[Jeffrey Yasskin, 2013-11-04 21:21:18]

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/server2/content_provider.py (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider.py:17: class
ContentInfo(object):
"TypedContent"? This isn't "Info" because it also includes the content.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider.py:27: '''Provides
correctly typed content: templates for HTML, text for CSS, binary
"Provides" is too vague: through what interface does it provide stuff? What are
the actual types callers should expect? You're breaking static type discipline
here, so you need to document this really clearly.

Also mention how it knows the content type of the files (filename, I think).

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider.py:34: file_system,
Document the difference between file_system and host_file_system.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider.py:42:
self._template_cache = compiled_fs_factory.ForTemplates(host_file_system)
This isn't used.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/server2/content_provider_test.py (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider_test.py:28: # Test file
system data which exercises the differnet mimetypes and different
sp: differnet

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider_test.py:29: # types of
redirects.
Why are redirects relevant to ContentProvider?

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider_test.py:67: #
TODO(kalman): Test supports_templates=False (json: supportsTemplates).
What's the "json:" note?

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider_test.py:77: def
_assertRedirect(self, redirect, permanent, content_info):
This isn't used.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/server2/content_providers.py (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_providers.py:19: '''Implements the
content_providers.json configuration. Returns
Document or link to documentation of the format of that file.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_providers.py:37: def
GetByServlet(self, path):
Docstring what this function means, as opposed to GetByName.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/server2/render_servlet.py (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/render_servlet.py:88: path =
path_within_content_provider
It looks like GetByServlet(path) returns |path| when it can't find a servlet, so
you might not need this else branch.

[not at google - send to devlin, 2013-11-04 23:34:49]

Long comment added to content_providers.json.

I also removed the "default" stuff since it wasn't used (the empty path thing
works fine).

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/server2/content_provider.py (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider.py:17: class
ContentInfo(object):
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> "TypedContent"? This isn't "Info" because it also includes the content.

Info can't include the content? I'd need to rename GetContentInfo.
GetTypedContent would be a weird name for that. GetContentAndType? So this could
be ContentAndType?

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider.py:27: '''Provides
correctly typed content: templates for HTML, text for CSS, binary
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> "Provides" is too vague: through what interface does it provide stuff? What
are
> the actual types callers should expect? You're breaking static type discipline
> here, so you need to document this really clearly.
> 
> Also mention how it knows the content type of the files (filename, I think).

Done.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider.py:34: file_system,
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> Document the difference between file_system and host_file_system.

Good point. It's only used by the template cache though; I'll delete it for now,
then add a comment in the follow-up which uses template cache.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider.py:42:
self._template_cache = compiled_fs_factory.ForTemplates(host_file_system)
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> This isn't used.

Done.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/server2/content_provider_test.py (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider_test.py:28: # Test file
system data which exercises the differnet mimetypes and different
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> sp: differnet

Done.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider_test.py:29: # types of
redirects.
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> Why are redirects relevant to ContentProvider?

Heh, I guess they were at some point. Not any more.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider_test.py:67: #
TODO(kalman): Test supports_templates=False (json: supportsTemplates).
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> What's the "json:" note?

That is appears as "supportsTemplates: false" in the JSON file. But I have no
idea why past-self thought that was relevant.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider_test.py:77: def
_assertRedirect(self, redirect, permanent, content_info):
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> This isn't used.

Ah. In a past life ContentProvider did redirects (and was called something
else). Hence this and the comment above about redirects.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/server2/content_providers.py (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_providers.py:19: '''Implements the
content_providers.json configuration. Returns
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> Document or link to documentation of the format of that file.

Done. Documentation added to content_providers.json.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_providers.py:37: def
GetByServlet(self, path):
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> Docstring what this function means, as opposed to GetByName.

Done.

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/server2/render_servlet.py (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/render_servlet.py:88: path =
path_within_content_provider
On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> It looks like GetByServlet(path) returns |path| when it can't find a servlet,
so
> you might not need this else branch.

Good point.

[Jeffrey Yasskin, 2013-11-04 23:45:20]

lgtm

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
File chrome/common/extensions/docs/server2/content_provider.py (right):

https://codereview.chromium.org/54603010/diff/1/chrome/common/extensions/docs...
chrome/common/extensions/docs/server2/content_provider.py:17: class
ContentInfo(object):
On 2013/11/04 23:34:49, kalman wrote:
> On 2013/11/04 21:21:18, Jeffrey Yasskin wrote:
> > "TypedContent"? This isn't "Info" because it also includes the content.
> 
> Info can't include the content? I'd need to rename GetContentInfo.
> GetTypedContent would be a weird name for that. GetContentAndType? So this
could
> be ContentAndType?

ContentAndType would be fine with me.

https://codereview.chromium.org/54603010/diff/100001/chrome/common/extensions...
File chrome/common/extensions/docs/server2/content_providers.py (right):

https://codereview.chromium.org/54603010/diff/100001/chrome/common/extensions...
chrome/common/extensions/docs/server2/content_providers.py:45: content_provider
is the ContentProvider with the longest "servlet" property
s/servlet/serveFrom/?

https://codereview.chromium.org/54603010/diff/100001/chrome/common/extensions...
File chrome/common/extensions/docs/templates/json/content_providers.json
(right):

https://codereview.chromium.org/54603010/diff/100001/chrome/common/extensions...
chrome/common/extensions/docs/templates/json/content_providers.json:11: // and
what URL the rule should match ("extensions/examples") aka "servlet".
s/servlet/serveFrom/?
Or maybe "serverUrlPath" or something similar that says how it's used, if you're
freshly naming this concept.

[not at google - send to devlin, 2013-11-05 01:05:41]

Committed patchset #5 manually as r232868.