New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 634092 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Remove duplicated Java classes from Cronet

Project Member Reported by xunji...@chromium.org, Aug 3 2016

Issue description

In Cronet, there are a few duplicated Java classes (ChromiumEffectiveConnectionType vs EffectiveConnectionType, LoadState vs UrlRequest.Status, UrlRequestError vs NetError).

For each auto-generated class, we manually create a Java class. The latter has JavaDocs whereas the former doesn't. This looks like a waste of resources. We should incorporate JavaDocs in the generated classes, and get rid of the manually created classes. The generated files should be added to Cronet's source jars, so our embedders have access to the JavaDocs as well. 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 5 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ea986397d77dc45b5b272ad68d00cc6063ee6987

commit ea986397d77dc45b5b272ad68d00cc6063ee6987
Author: xunjieli <xunjieli@chromium.org>
Date: Fri Aug 05 13:39:59 2016

Modify java_cpp_enum.py to preserve comments

This CL modifies java_cpp_enum.py to preserve comments on the
enums in the generated Java file.

The comments are useful for developers who usually read the
JavaDoc and not the C++ files.

BUG= 634092 

Review-Url: https://codereview.chromium.org/2210633002
Cr-Commit-Position: refs/heads/master@{#410047}

[modify] https://crrev.com/ea986397d77dc45b5b272ad68d00cc6063ee6987/build/android/gyp/java_cpp_enum.py
[modify] https://crrev.com/ea986397d77dc45b5b272ad68d00cc6063ee6987/build/android/gyp/java_cpp_enum_tests.py

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cfb2180def7b0e17bd4dce2bc8300fb4ea02e4ba

commit cfb2180def7b0e17bd4dce2bc8300fb4ea02e4ba
Author: xunjieli <xunjieli@chromium.org>
Date: Mon Aug 15 17:13:50 2016

Include generated EffectiveConnectionType.java in cronet api jar and JavaDoc

This CL includes the EffectiveConnectionType.java in cronet
api jar and in JavaDoc, since that is a part of Cronet API.

BUG= 634092 

Review-Url: https://codereview.chromium.org/2234113002
Cr-Commit-Position: refs/heads/master@{#411977}

[modify] https://crrev.com/cfb2180def7b0e17bd4dce2bc8300fb4ea02e4ba/components/cronet/android/BUILD.gn
[modify] https://crrev.com/cfb2180def7b0e17bd4dce2bc8300fb4ea02e4ba/components/cronet/tools/generate_javadoc.py
[modify] https://crrev.com/cfb2180def7b0e17bd4dce2bc8300fb4ea02e4ba/components/cronet/tools/jar_src.py

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f84d14ad945d5cc3d945bf4df0d9ca9e82824ed7

commit f84d14ad945d5cc3d945bf4df0d9ca9e82824ed7
Author: bsep <bsep@chromium.org>
Date: Mon Aug 15 17:58:53 2016

Revert of Include generated EffectiveConnectionType.java in cronet api jar and JavaDoc (patchset #5 id:120001 of https://codereview.chromium.org/2234113002/ )

Reason for revert:
compile failed on Android Arm64:

FAILED: gen/components/cronet/android/generate_javadoc.d
python ../../components/cronet/tools/generate_javadoc.py --output-dir cronet --input-dir ../../components/cronet --overview-file cronet/README.md.html --readme-file ../../components/cronet/README.md --depfile gen/components/cronet/android/generate_javadoc.d --lib-java-dir lib.java/components/cronet/android --input-src-jar cronet/cronet_api-src.jar
Traceback (most recent call last):
  File "../../components/cronet/tools/generate_javadoc.py", line 85, in <module>
    sys.exit(main())
  File "../../components/cronet/tools/generate_javadoc.py", line 73, in main
    GenerateJavadoc(options, os.path.abspath(unzipped_jar_path))
  File "../../components/cronet/tools/generate_javadoc.py", line 48, in GenerateJavadoc
    raise build_utils.CalledProcessError(working_dir, javadoc_cmd, stdout)

Original issue's description:
> Include generated EffectiveConnectionType.java in cronet api jar and JavaDoc
>
> This CL includes the EffectiveConnectionType.java in cronet
> api jar and in JavaDoc, since that is a part of Cronet API.
>
> BUG= 634092 
>
> Committed: https://crrev.com/cfb2180def7b0e17bd4dce2bc8300fb4ea02e4ba
> Cr-Commit-Position: refs/heads/master@{#411977}

TBR=mef@chromium.org,kapishnikov@chromium.org,xunjieli@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 634092 

Review-Url: https://codereview.chromium.org/2242243002
Cr-Commit-Position: refs/heads/master@{#411980}

[modify] https://crrev.com/f84d14ad945d5cc3d945bf4df0d9ca9e82824ed7/components/cronet/android/BUILD.gn
[modify] https://crrev.com/f84d14ad945d5cc3d945bf4df0d9ca9e82824ed7/components/cronet/tools/generate_javadoc.py
[modify] https://crrev.com/f84d14ad945d5cc3d945bf4df0d9ca9e82824ed7/components/cronet/tools/jar_src.py

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3d60df9ba5010441b73b5878ec3ff0fb9056a6a3

commit 3d60df9ba5010441b73b5878ec3ff0fb9056a6a3
Author: xunjieli <xunjieli@chromium.org>
Date: Tue Aug 16 17:17:40 2016

Include generated EffectiveConnectionType.java in cronet api jar and JavaDoc

This CL includes the EffectiveConnectionType.java in cronet
api jar and in JavaDoc, since that is a part of Cronet API.

BUG= 634092 

Committed: https://crrev.com/cfb2180def7b0e17bd4dce2bc8300fb4ea02e4ba
Review-Url: https://codereview.chromium.org/2234113002
Cr-Original-Commit-Position: refs/heads/master@{#411977}
Cr-Commit-Position: refs/heads/master@{#412270}

[modify] https://crrev.com/3d60df9ba5010441b73b5878ec3ff0fb9056a6a3/components/cronet/android/BUILD.gn
[modify] https://crrev.com/3d60df9ba5010441b73b5878ec3ff0fb9056a6a3/components/cronet/android/api/build.xml
[modify] https://crrev.com/3d60df9ba5010441b73b5878ec3ff0fb9056a6a3/components/cronet/tools/generate_javadoc.py
[modify] https://crrev.com/3d60df9ba5010441b73b5878ec3ff0fb9056a6a3/components/cronet/tools/jar_src.py

Status: Fixed (was: Started)
LoadState and NetError use cpp enum template because they need preprocessing. I didn't find a way to switch them over to use java_cpp_enum.py. I guess we will live with two copies of these two classes for a while until that part is figured out :( 

Sign in to add a comment