Re: Can we turn CSSStyleDeclaration attributes into valid IDL attributes?

[Ian Kilpatrick]

More appropriate team would be style-dev@.

Ian

On Tue, Dec 17, 2019 at 3:43 AM Yuki Shiino <yukis...@chromium.org> wrote:

Thanks for the question.  :)

Would it have memory implications (use much more memory because all of the properties and their aliases will always be there)?

I don't think that memory would be a huge problem.  However, if it would be a problem, bindings team is happy to help.

Unless you actually instantiate a CSSStyleDeclaration, Blink-V8 bindings doesn't create an interface object nor prototype object nor any attributes.  Unless you actually use an attribute, V8 does not instantiate an accessor function.  There are several mechanisms that are lazy.  So, "all of the properties ... will always be there" is not true.

Also, those JS properties of IDL attributes are created per v8::Context (per window).  Even if you created 1,000 instances of CSSStyleDeclaration on your page, we don't create 1,000 times properties.  1,000 instances share one set of properties of CSSStyleDeclaration.

2019年12月17日(火) 17:50 PhistucK <phis...@gmail.com>:

Would it have memory implications (use much more memory because all of the properties and their aliases will always be there)?

☆PhistucK

On Tue, Dec 17, 2019 at 8:38 AM Yuki Shiino <yukis...@chromium.org> wrote:

Hi layout-dev@,

Is there any chance for layout-dev@ to work on https://crbug.com/700338 ?

Roughly speaking, the issue is that CSSStyleDeclaration's IDL attributes are not implemented as IDL attributes.  They're implemented as "named properties".  It'd be a sort of abuse of "named properties".

I heard that the origin trial team is now working to support CSSStyleDeclaration [1][2] just because of the issue above, despite that we already have good support of origin trial for valid IDL attributes.  It seems to me that the current implementation of CSSStyleDeclaration is unnecessarily introducing complexity and spec violations.

It seems to me that we'd better fix the issue https://crbug.com/700338 before it's getting worse and worse.

[1] Design doc

https://docs.google.com/document/d/1Lc3U2WV38GZ3ShTAm3-p6fbZeGJGkxL-KZfWIQ-p8GU/edit?usp=sharing

[2] 4 patches starting the below one, and maybe more patches.

https://chromium-review.googlesource.com/c/chromium/src/+/1934648

-- 

Yuki Shiino

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAN0uC_QSVyWcmdA%2Bid%3DB6EfNkkiqg3RwmwdZnLt1%3Dp6maO833g%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAN0uC_T__3WxSnP5C223DNA8BOFQY7tYW-eKQG2m_np7ethe3Q%40mail.gmail.com.

[Anders Hartvoll Ruud]

Sure, I'll take a look at this.

Our bindings/build system will need to support generated IDL-files. I'm not sure if we do this somewhere already?

You received this message because you are subscribed to the Google Groups "style-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to style-dev+...@chromium.org.
To view this discussion on the web, visit https://groups.google.com/a/chromium.org/d/msgid/style-dev/CAJL3UpS%2BMQku92Ty7pJF2irA1Vvmtf8anw3H-vUp5dntngNmFQ%40mail.gmail.com.

[Chris Nardi]

I had a very-WIP patch from a year ago working on this at https://chromium-review.googlesource.com/c/chromium/src/+/1077010.

To view this discussion on the web, visit https://groups.google.com/a/chromium.org/d/msgid/style-dev/CAKFBnUo7gDzpFZRs6q8usp_3PbSexBPjWR3fXUL-raiPYO_u%3DQ%40mail.gmail.com.

[Yuki Shiino]

Huge thanks for starting the work on this.  :D

2019年12月18日(水) 17:09 Chris Nardi <cna...@chromium.org>:

[Jeremy Roman]

Watch out for Android binary size regressions here. I think it's quite likely to regress significantly. This can probably be mitigated with some cleverness (making sure the generated functions end up as short tail calls under our supported ABIs, for instance), but anything that gets stamped out 1000x tends to hit binary size.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAN0uC_QxuoDbWi2AptN5bpNLWO3OOwndQTwtr5afUE4m34CH6A%40mail.gmail.com.

[Yuki Shiino]

Agreed with Jeremy.  If the binary size matters a lot, let's optimize it.  tkent@ recently used a certain technique and reduced the binary size of bindings (I think this is the same as Jeremy's idea).  I think we can use the same technique for CSSStyleDeclaration, too.  Also we can introduce something like [CallWith=PropertyName] if it helps a lot.  Let's keep an eye on it.

2019年12月19日(木) 0:15 Jeremy Roman <jbr...@chromium.org>:

[Anders Hartvoll Ruud]

*Many months later*

Picking this up again. If we don't do anything special, binary size increases with 300K [1], which is a lot just for this. So we might want to explore the [CallWith=PropertyName] approach.

Also, cssom requires both myAttribute and MyAttribute, which the bindings generator can't handle. If we add [CallWith=PropertyName], we can probably get away with not fixing that, though.

[1] WIP CL (trybots not supposed to pass yet) | SuperSize HTML Diff

[Yuki Shiino]

Thanks for the news.  It's really good to know and insightful.

I agree with your idea.  I think we'd like to have an optimization hack similar to 'onxxx' event handlers, which would bloat the binary size if no hack.  [CallWith=PropertyName] should be necessary.

Unfortunately, we're out of bandwidth in Q2.  Let us think of the size reduction after Q2.

Cheers,

Yuki Shiino

2020年4月27日(月) 22:21 Anders Hartvoll Ruud <and...@chromium.org>:

[Anders Hartvoll Ruud]

On Tue, Apr 28, 2020 at 10:07 AM Yuki Shiino <yukis...@chromium.org> wrote:

Thanks for the news.  It's really good to know and insightful.

I agree with your idea.  I think we'd like to have an optimization hack similar to 'onxxx' event handlers, which would bloat the binary size if no hack.  [CallWith=PropertyName] should be necessary.

Any rough idea on how you would implement this?

Note that my WIP CL has two major "areas" of bloat.

•  v8_css_style_declaration.cc, i.e. the generated bindings. Adds 256kB (~82%).
•  css_style_declaration_attributes.cc, i.e. generated functions (called directly by bindings) that wrap CSSStyleDeclaration::setProperty/getPropertyValue. Adds 52kB (~17%).

In other words, the bindings code itself is the biggest issue. We need a mechanism for sharing more code in the generated bindings.

[Yuki Shiino]

What in my head now is:

void Callback(const v8::FunctionCallbackInfo<v8::Value>& info) {

  const char* property_name = "property_name";

  DoEverythingElseForCSSStyleDeclaration(info, property_name);

}

Cheers,

Yuki Shiino

2020年4月28日(火) 22:36 Anders Hartvoll Ruud <and...@chromium.org>:

[Anders Hartvoll Ruud]

On Tue, Apr 28, 2020 at 4:02 PM Yuki Shiino <yukis...@chromium.org> wrote:

What in my head now is:

void Callback(const v8::FunctionCallbackInfo<v8::Value>& info) {

  const char* property_name = "property_name";

  DoEverythingElseForCSSStyleDeclaration(info, property_name);

}

Cheers,

Yuki Shiino

I was thinking along those lines too. Makes sense.

The CL is now finished (although with a temporary workaround for Issue 1075981), however, I won't move forward with it until we can make the footprint more acceptable (Issue 1075980). So for now I'm putting this on ice. (Again).

[Yuki Shiino]

Thanks for all the trials and investigations.  Once we'll implement something that reduces the binary size, we'll update this thread.  :)

2020年4月29日(水) 22:04 Anders Hartvoll Ruud <and...@chromium.org>: