IIUC there is a discussion about naming style and I'd prefer to have consensus on naming convention rather than fixing random things.

https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=footer#!msg/platform-architecture-dev/wtEU7zQUsQg/YnWOg6SbAAAJ

Could you elaborate why do you want to change the name? If this is related to style consistency why do we change this only?

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Element.h?type=cs&q=openshadowroot&sq=package:chromium&l=475

There are several other functions to get a shadow root which are shown above.
ClosedShadowRoot(), AuthorShadowRoot() are the examples.
Only openShadowRoot() has a lower letter and I thought this should be changed to the capital letter.

Then should we change createShadow()/attachShadow() as well? My point is that we should have a consistent naming convention.

#2
OpenShadowRoot is not a web API method, so it should be capitalized in our new naming convention.
(createShadowRoot and attachShadowRoot are web APIs.)
I guess some methods were left uncapitalized in the big re-format, because they needed changes in our code generator.

I don't have strong opinion so I'm fine with fixing this.

(Though I do think it's a web exposed API because my mental model of ImplementedAs is just an alias.)

https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/blink-c++.md#naming
says "As an exception, method names for web-exposed bindings begin with a lowercase letter to match JavaScript."
so I think alias does not match JavaScript, and should be capitalized like other methods.

Fair enough and I don't want to mess up this issue but one thing...

Are you saying:
- Getters with ImplementedAs -> it should be capitalized
- Getters without ImplementedAs -> it should be uncapitalized

?

This is confusing to me.

Assuming web exposed methods in JS are uncapitalized, IIUC,
- Getters with ImplementedAs -> the callee method's name (=ImplementedAs) does not match with web exposed name, so the callee should be capitalized.
- Getters without ImplementedAs -> the callee method's name is same as web exposed name, so they should be uncapitalized.

I'm just saying the rule is confusing to me :)

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

commit 91530e3395cc049dbbc2876251cd2697571b12ad
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Fri Aug 18 03:49:01 2017

bindings: Do not change attribute's name if ImplementedAs applied

Bug:  756339 
Change-Id: I2874c90f5f191543d4adaea8ec13d45f0133b032
Reviewed-on: https://chromium-review.googlesource.com/618078
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495453}
[modify] https://crrev.com/91530e3395cc049dbbc2876251cd2697571b12ad/third_party/WebKit/Source/bindings/scripts/v8_attributes.py
[modify] https://crrev.com/91530e3395cc049dbbc2876251cd2697571b12ad/third_party/WebKit/Source/bindings/tests/idls/core/TestInterface.idl
[modify] https://crrev.com/91530e3395cc049dbbc2876251cd2697571b12ad/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp
[modify] https://crrev.com/91530e3395cc049dbbc2876251cd2697571b12ad/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.h

Sorry for chiming in late, but my understanding is that:

If idl is directly reflected in C++ interface -> uncapitalize
If idl specifies some identifier name as "ImplementedAs" -> use the name as is

"ImplementedAs=" name is just an C++ interface name, so I'd like binding generator use the symbol name as is, without any capitilzation/uncapitalization.
There should be both cases.

#13
Yes, and the code generator behaves as you noted for "ImplementedAs" case.
I'm sorry that my explanation was confusing.

IMHO, 'uncapitalize' is a wrong way. Whichever "ImeplementedAs" is specified or not, we should use the attribute's name (or name specified in ImplementedAs) by default.

Just for record, the situation isn't as easy as "just treat the value of ImplementedAs as-is". What kind of naming style we should use for getter?

// .idl
[ImplementedAs=fooBar] attribute long foo; // Using uncapitalized name as 

// .h
long setFooBar(); // ?
long setfooBar(); // ?
long SetFooBar(); // ?

Please see the discussion on https://bugs.chromium.org/p/chromium/issues/detail?id=673039.

That's why I want to have an agreement on naming style before modifying random things.

I didn't notice the case for setters... I assumed only readonly ones.
Yes, there is ambiguity and in the current Blink coding style, SetFooBar() is
the recommended style, isn't it?

Specifically at https://bugs.chromium.org/p/chromium/issues/detail?id=673039#c37

+cc: dcheng@ if you have something.

There was a discussion that there are several pros to use the different naming style only for IDL-rooted names (bindings names) than Blink's (or Chromium's) standard naming convention.

IIRC, dcheng@ supported to continue using "setTimeout" in Blink C++ code for Window interface's "setTimeout" IDL operation.


> SetFooBar() is the recommended style, isn't it?

No, not for IDL-rooted bindings name.
At least, we once agreed to use the different naming convention for bindings.

I see, I don't have any strong opinion for either side for the case which
still need some prefix (set, Set or even set_) for the specified ImplementedAs
name, I'll follow the coding convention.

So I guess it's complicated. When we did the rewrite, we scraped the IDL files and avoided renaming anything that was pointed to by ImplementedAs to avoid making things more complex. So everything that was ImplementedAs kept the legacy name (which is lowerCamelCase).

> IIRC, dcheng@ supported to continue using "setTimeout" in Blink C++ code for Window interface's "setTimeout" IDL operation.

This is named "setTimeout" because that's how the function is specified in the HTML spec: https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-settimeout

For attribute setters, it seems like we also use lowerCamelCase: for example, see Document.domain here: https://cs.chromium.org/chromium/src/out/Debug/gen/blink/bindings/core/v8/V8Document.cpp?rcl=07237c8fd68bf051ce5f6b80adb13e30ed3b9efb&l=381. This seems fine.

As for ImplementedAs, we should use the name literally. The bindings are already complicated as it is, and transforming the name would make it even more magical. Whether we want these uppercase or lowercase, I don't feel strongly about: mostly it just seems like we need to make sure we have a convention set.