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

Issue 602503 link

Starred by 3 users

Issue metadata

Status: Duplicate
Merged: issue 617324
Owner:
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Isolate Java code in content/ and chrome/ via public/ API

Project Member Reported by aelias@chromium.org, Apr 12 2016

Issue description

Java code in content/ and chrome/ directories can currently talk to the other side without needing to go through public/.  This also indirectly allows C++ code to violate the public/ API.  We should start enforcing the same DEPS rules on all our Java code that we do to C++, and resolve all the violations that we find (which will likely involve introducing new abstractions).
 

Comment 1 by aelias@chromium.org, Apr 12 2016

Cc: changwan@chromium.org
Just to confirm that DEPS file can be used for Java to help isolate them and clarify dependency, as we have a prescan pass that reads all the classes and builds a mapping of class name -> filepath. In java_checker.CheckFile() each import statement is converted to a real file path and checked against the rules in the DEPS files.

buildtools/checkdeps/java_checker.py 

A couple of observations:

1) With the include_rules in https://code.google.com/p/chromium/codesearch#chromium/src/chrome/android/java/DEPS taking effect, java classes' in chrome/ have well-enforced dependency on content/public/android/java, numerous components/, and sync/android/java/src/org/chromium/sync. The first 2 look okay, while sync/ looks a bit misplaced. I think it better be somewhere under chrome/. There may be a history related to this, which I'm not aware of.

2) I see  content/public java classes are grouped into two - content/public/android/java/src/org/chromium/{content_public, content}. If content_public is there to define the API for other apps like chrome/ to link content module to, I think the clearer distinction is necessary. Only the former should be referenced by chrome/ as API, leaving the latter as inaccessible implementation detail. That would be more aligned with c++ header/abstract class files in content/public/ vs. internal impl classes under other directories in content/.

Comment 4 by aelias@chromium.org, Apr 26 2016

Cc: aelias@chromium.org
Mergedinto: 617324
Status: Duplicate (was: Assigned)

Sign in to add a comment