content java api

8 views
Skip to first unread message

Bo Liu

unread,
Dec 1, 2016, 8:56:33 PM12/1/16
to Alexandre Elias, dtra...@chromium.org, ted...@chromium.org, yfri...@chromium.org, Slimming Clank
Wrote some discussion points on public api in a doc: https://docs.google.com/a/chromium.org/document/d/15EwQXNuO_P0aojTNmT44c4O-ssaFHydJLBqKNLMU-9o/edit?usp=sharing

Added all the content android owners here. Thoughts? More things to add/discuss? Doc is editable by chromium. I'm hoping this will eventually turn into an android/java section on the content api wiki page.

This should be relevant for the ContentViewCore refactor as well, as newly created classes should respect the api boundary

Bo Liu

unread,
Dec 5, 2016, 4:33:14 PM12/5/16
to Alexandre Elias, dtra...@chromium.org, ted...@chromium.org, yfri...@chromium.org, Slimming Clank
bump for this one

If no one has strong feelings by say end of week, I'll just write up more concrete rules and base my decision on any discussions in the doc, and my personal opinions :p

Bo Liu

unread,
Dec 15, 2016, 6:01:51 PM12/15/16
to Alexandre Elias, dtra...@chromium.org, ted...@chromium.org, yfri...@chromium.org, Slimming Clank
Summarized results of yesterday's meeting the doc. However, let's keep the discussion in the email thread, rather than in the doc. Using doc for discussion doesn't work very well, because only I get notified of the comments. So here's pasting the new content from the doc, and let's discuss away!

Rules vs guidelines

Rule should be simple. We commit to fix any existing code that violates any rules.

Guidelines are less strict. Violations are allowed


Rules

Interface implemented by embedder (eg WebContentsObserver) should be Java Interface. Put default implementations in a component, eg WebContentsDelegateAndroid

  • What about content shell? What implementation will content shell use?


Use Java Interface for interfaces implemented by content (eg WebContents). If a static factory is required, define a separate abstract class with only static methods.


Allow @CalledByNative. Disallow native methods.

  • Only native method is in LoadUrlParams right now


Guideline:

Avoid implementation in content_public.

Avoid android-only interfaces, or android-only methods on those interfaces. Interfaces (both ones implemented by embedder, and ones implemented by content) should match native counterparts.

Do not use ScopedJavaGlobalRef to interfaces implemented by embedder.

  • Make this a rule?

David Trainor

unread,
Dec 15, 2016, 6:11:03 PM12/15/16
to bo...@chromium.org, Alexandre Elias, ted...@chromium.org, yfri...@chromium.org, Slimming Clank
This is great thanks Bo!

On Thu, Dec 15, 2016 at 3:01 PM Bo Liu <bo...@chromium.org> wrote:
Summarized results of yesterday's meeting the doc. However, let's keep the discussion in the email thread, rather than in the doc. Using doc for discussion doesn't work very well, because only I get notified of the comments. So here's pasting the new content from the doc, and let's discuss away!

Rules vs guidelines

Rule should be simple. We commit to fix any existing code that violates any rules.

Guidelines are less strict. Violations are allowed


Rules

Interface implemented by embedder (eg WebContentsObserver) should be Java Interface. Put default implementations in a component, eg WebContentsDelegateAndroid

  • What about content shell? What implementation will content shell use?


Should we pick a general shared support component for all android/content/public embedder support classes like this?

I think content shell should try to use the support implementation if possible, but if not it can just implement the interface itself.  I think this would just be a place to put content/public shared implementations or other common code that can be leveraged by multiple embedders.

Bo Liu

unread,
Dec 15, 2016, 6:32:21 PM12/15/16
to David Trainor, Alexandre Elias, ted...@chromium.org, yfri...@chromium.org, Slimming Clank
On Thu, Dec 15, 2016 at 3:10 PM, David Trainor <dtra...@chromium.org> wrote:
This is great thanks Bo!

On Thu, Dec 15, 2016 at 3:01 PM Bo Liu <bo...@chromium.org> wrote:
Summarized results of yesterday's meeting the doc. However, let's keep the discussion in the email thread, rather than in the doc. Using doc for discussion doesn't work very well, because only I get notified of the comments. So here's pasting the new content from the doc, and let's discuss away!

Rules vs guidelines

Rule should be simple. We commit to fix any existing code that violates any rules.

Guidelines are less strict. Violations are allowed


Rules

Interface implemented by embedder (eg WebContentsObserver) should be Java Interface. Put default implementations in a component, eg WebContentsDelegateAndroid

  • What about content shell? What implementation will content shell use?


Should we pick a general shared support component for all android/content/public embedder support classes like this?

Yes. That's what I meant to say. Updated the doc, with more questions:

Interface implemented by embedder (eg WebContentsObserver) should be Java Interface. Put default implementations in a component.

  • Name the component. content_support? Content_support_android?

  • Note current WebContentsDelegateAndroid isn’t quite this pattern, but can certainly be bent into this pattern.

 

I think content shell should try to use the support implementation if possible, but if not it can just implement the interface itself.  I think this would just be a place to put content/public shared implementations or other common code that can be leveraged by multiple embedders.

Maybe..

Content in general is not allowed to depend on components (with exceptions): https://cs.chromium.org/chromium/src/content/DEPS?rcl=0&l=23

However shell can include more components: https://cs.chromium.org/chromium/src/content/shell/DEPS?rcl=0&l=26

So I think so?
 


Use Java Interface for interfaces implemented by content (eg WebContents). If a static factory is required, define a separate abstract class with only static methods.


Allow @CalledByNative. Disallow native methods.

  • Only native method is in LoadUrlParams right now


Guideline:

Avoid implementation in content_public.

Avoid android-only interfaces, or android-only methods on those interfaces. Interfaces (both ones implemented by embedder, and ones implemented by content) should match native counterparts.

Do not use ScopedJavaGlobalRef to interfaces implemented by embedder.

  • Make this a rule?

On Mon, Dec 5, 2016 at 1:32 PM, Bo Liu <bo...@chromium.org> wrote:
bump for this one

If no one has strong feelings by say end of week, I'll just write up more concrete rules and base my decision on any discussions in the doc, and my personal opinions :p

On Thu, Dec 1, 2016 at 5:55 PM, Bo Liu <bo...@chromium.org> wrote:
Wrote some discussion points on public api in a doc: https://docs.google.com/a/chromium.org/document/d/15EwQXNuO_P0aojTNmT44c4O-ssaFHydJLBqKNLMU-9o/edit?usp=sharing

Added all the content android owners here. Thoughts? More things to add/discuss? Doc is editable by chromium. I'm hoping this will eventually turn into an android/java section on the content api wiki page.

This should be relevant for the ContentViewCore refactor as well, as newly created classes should respect the api boundary


--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clank+unsubscribe@chromium.org.
To post to this group, send email to slimmin...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/slimming-clank/CAAUoNbNPhPMqy5KFMbWcxeBgsMS1g6McGgSLtU9O5r75as%2BnwA%40mail.gmail.com.

David Trainor

unread,
Dec 15, 2016, 6:50:35 PM12/15/16
to bo...@chromium.org, Alexandre Elias, ted...@chromium.org, yfri...@chromium.org, Slimming Clank
On Thu, Dec 15, 2016 at 3:32 PM Bo Liu <bo...@chromium.org> wrote:
On Thu, Dec 15, 2016 at 3:10 PM, David Trainor <dtra...@chromium.org> wrote:
This is great thanks Bo!

On Thu, Dec 15, 2016 at 3:01 PM Bo Liu <bo...@chromium.org> wrote:
Summarized results of yesterday's meeting the doc. However, let's keep the discussion in the email thread, rather than in the doc. Using doc for discussion doesn't work very well, because only I get notified of the comments. So here's pasting the new content from the doc, and let's discuss away!

Rules vs guidelines

Rule should be simple. We commit to fix any existing code that violates any rules.

Guidelines are less strict. Violations are allowed


Rules

Interface implemented by embedder (eg WebContentsObserver) should be Java Interface. Put default implementations in a component, eg WebContentsDelegateAndroid

  • What about content shell? What implementation will content shell use?


Should we pick a general shared support component for all android/content/public embedder support classes like this?

Yes. That's what I meant to say. Updated the doc, with more questions:

Interface implemented by embedder (eg WebContentsObserver) should be Java Interface. Put default implementations in a component.

  • Name the component. content_support? Content_support_android?


Maybe content_support makes sense and we just have an android/ folder in there for now?  I don't feel strongly either way though... 

  • Note current WebContentsDelegateAndroid isn’t quite this pattern, but can certainly be bent into this pattern.


Yeah when I looked I was surprised at how much stuff there was in the web_contents_delegate_android component.  For some reason I just expected WebContentsDelegateAndroid.java... although in hindsight that wouldn't make sense without those other pieces.  It seems to still fit our criteria (e.g. there are classes like ColorPickerAndroid, but they are things that all content embedders want, etc.).

 

I think content shell should try to use the support implementation if possible, but if not it can just implement the interface itself.  I think this would just be a place to put content/public shared implementations or other common code that can be leveraged by multiple embedders.

Maybe..

Content in general is not allowed to depend on components (with exceptions): https://cs.chromium.org/chromium/src/content/DEPS?rcl=0&l=23

However shell can include more components: https://cs.chromium.org/chromium/src/content/shell/DEPS?rcl=0&l=26

So I think so?

Yeah I feel like (and let me know if I'm wrong) the shell is just a super light weight embedder of the content layer, so it should be able to pull in what it needs like any other embedder would.
 
 


Use Java Interface for interfaces implemented by content (eg WebContents). If a static factory is required, define a separate abstract class with only static methods.


Allow @CalledByNative. Disallow native methods.

  • Only native method is in LoadUrlParams right now


Guideline:

Avoid implementation in content_public.

Avoid android-only interfaces, or android-only methods on those interfaces. Interfaces (both ones implemented by embedder, and ones implemented by content) should match native counterparts.

Do not use ScopedJavaGlobalRef to interfaces implemented by embedder.

  • Make this a rule?

On Mon, Dec 5, 2016 at 1:32 PM, Bo Liu <bo...@chromium.org> wrote:
bump for this one

If no one has strong feelings by say end of week, I'll just write up more concrete rules and base my decision on any discussions in the doc, and my personal opinions :p

On Thu, Dec 1, 2016 at 5:55 PM, Bo Liu <bo...@chromium.org> wrote:
Wrote some discussion points on public api in a doc: https://docs.google.com/a/chromium.org/document/d/15EwQXNuO_P0aojTNmT44c4O-ssaFHydJLBqKNLMU-9o/edit?usp=sharing

Added all the content android owners here. Thoughts? More things to add/discuss? Doc is editable by chromium. I'm hoping this will eventually turn into an android/java section on the content api wiki page.

This should be relevant for the ContentViewCore refactor as well, as newly created classes should respect the api boundary


--
You received this message because you are subscribed to the Google Groups "Slimming Clank" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slimming-clan...@chromium.org.
Reply all
Reply to author
Forward
0 new messages