Side by Side Diff: chrome/browser/android/webapk/DEPS
Issue 2138973002:
Initial CL for talking to the WebAPK server to generate WebAPK (Closed)
Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set:
Created 4 years, 5 months ago
Use n/p to move between diff chunks;
N/P to move between comments.
Draft comments are only viewable by you.
On 2016/07/13 00:24:01, Yaron wrote:
> does this need to be moved to content/public or are there examples of
unittests
> which reach into content (I'm unsure)
bump.
looking at the test, I don't think it's testing the right thing.
content::ManifestParser isn't even going to really parse the manifest (the
server will) and is orthogonal to your functionality.
pkotwicz
2016/07/20 05:13:41
You are right using content/renderer/manifest/mani
You are right using content/renderer/manifest/manifest_parser.h in the unit test
is wrong. The spirit of the test is correct. The server API is defined in terms
of the W3C Web Manifest spec. Testing WebApkBuilder::OrientationToString()
against ManifestParser is valid. Perhaps I should introduce ManifestUtil in
content/public/manifest_util.h which converts between
blink::WebScreenOrientationLockType <-> std::string. What do you think?
Yaron
2016/07/20 13:13:50
That would be fine with me. Another option would b
On 2016/07/20 05:13:41, pkotwicz wrote:
> You are right using content/renderer/manifest/manifest_parser.h in the unit
test
> is wrong. The spirit of the test is correct. The server API is defined in
terms
> of the W3C Web Manifest spec. Testing WebApkBuilder::OrientationToString()
> against ManifestParser is valid. Perhaps I should introduce ManifestUtil in
> content/public/manifest_util.h which converts between
> blink::WebScreenOrientationLockType <-> std::string. What do you think?
That would be fine with me. Another option would be to move the enum conversion
code down to content (with a public accessor) and test it there. It's nice in
that it localizes it with the relevant code as opposed to the mapping being in a
consumer and it's not dependent on our specific feature.
Issue 2138973002: Initial CL for talking to the WebAPK server to generate WebAPK
(Closed)
Created 4 years, 5 months ago by pkotwicz
Modified 4 years, 4 months ago
Reviewers: mlamouri (slow - plz ping), gone, dominickn, Xi Han, Robert Sesek, scottkirkwood, ScottK, Yaron, no sievers
Base URL: https://chromium.googlesource.com/chromium/src.git@master
Comments: 146