LGTM2

On 3/26/24 1:27 PM, Yoav Weiss (@Shopify) wrote:
LGTM1

On Monday, March 25, 2024 at 4:42:55 PM UTC+1 Mike Taylor wrote:

    Thank you for the answers Etienne. Once the Testing bit has been
    requested, I'm happy to give an LGTM.

    On 3/25/24 11:08 AM, Etienne Pierre-doray wrote:

        Right - that's my question. I also asked how similar this
        change is to whatever Safari implements - do they also align
        wakeups of JS timers for cross-origin, < 75% of viewport,
        frames that haven't yet received a user gesture? You
        previously wrote "non-interacted cross origin frames" for
        WebKit. Is there also a viewport condition?

    I don't think WebKit has a viewport condition. [code
    
<https://github.com/WebKit/WebKit/blob/main/Source/WebCore/dom/Document.cpp#L4005>]

        I believe the question Mike asked is not if this is spec
        compliant, but if the specifics of this should be more
        tightly specified, given that both WebKit and Chromium find a
        similar intervention useful.

    Defining this concept in the spec would presumably be used to
    more strictly define when throttling is allowed (1) or mandatory
    (2) by browsers. I really don't think it's useful to specify this
    more tightly in the spec, as it makes performance interventions
    overly restrictive and is prone to rotting over time, as common
    patterns and APIs evolve.
    1- Allowing throttling *only* for some definition of unimportant
    frames could provide a more predictable platform to web devs, but
    it seems overly restrictive on what the platform is allowed to
    do, and locks us in wrt. the kind of performance intervention
    we're allowed to do. This is already not true: both Webkit and
    blink implement various throttling in power-reducing situations
    other than unimportant frames.
    2- It's unclear how making throttling mandatory in certain
    situations brings value to web devs, especially if the
    intervention is different among browsers. It does however add
    unnecessary constraints on the platform.
    As a case in point, the spec specifies a mandatory 4ms delay on
    settimeout (8.6.5
    
<https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timers>)
    with a nesting level greater than 5. This intervention doesn't
    really make sense in modern day browsers, and neither blink or
    webkit are spec compliant (in distinct ways).


    On Fri, Mar 22, 2024 at 2:57 PM Mike Taylor
    <miketa...@chromium.org> wrote:

        On 3/22/24 1:37 AM, Yoav Weiss (@Shopify) wrote:


        On Thu, Mar 21, 2024 at 10:11 AM Zheda Chen
        <zheda.c...@intel.com> wrote:

            The privacy/security/enterprise/debuggability gates are
            requested on Chrome Status. Testing gate to be requested
            later.

        Thanks - we've been asked to not send LGTMs until all bits
        have been requested. Can you let us know when Testing is?


            "Unimportant" cross origin frames means they are
            cross-origin, visible but use non-large proportion
            (<75%) of page's visible area and have not received a
            user gesture. All 3 conditions should be met. The
            criteria are too long so we use "unimportant" concept here.

        Thank you - that's much more clear to me now.


            This intervention is spec-compliant. The spec mentions
            "the SetTimeout/SetInterval API does not guarantee that
            timers will run exactly on schedule. Delays due to CPU
            load, other tasks, etc, are to be expected". The wait
            length of time is implementation-defined, "which is
            intended to allow user agents to pad timeouts as needed
            to optimize the power usage of the device". In Safari,
            DOM timers of non-interacted cross origin frames are
            aligned to 30ms after reaching max nesting level (>=5).
            In Firefox, all DOM timers of frames (both same origin
            and cross origin) are aligned to 16ms. See details
            in"Interoperability and Compatibility Risks", "Safari
            views", "Firefox views".


        I believe the question Mike asked is not if this is spec
        compliant, but if the specifics of this should be more
        tightly specified, given that both WebKit and Chromium find
        a similar intervention useful.
        Right - that's my question. I also asked how similar this
        change is to whatever Safari implements - do they also align
        wakeups of JS timers for cross-origin, < 75% of viewport,
        frames that haven't yet received a user gesture? You
        previously wrote "non-interacted cross origin frames" for
        WebKit. Is there also a viewport condition?

            On Thursday, March 21, 2024 at 1:27:16 AM UTC+8
            mike...@chromium.org wrote:

                You should be able to see all the various bits for
                approvals in your chromestatus entry now, can you
                fill them out please?

                There have been a few questions/comments about the
                lack of clarity of what "unimportant cross-origin
                frames" are. What exactly is the definition? You
                mention that Safari has a similar intervention - how
                similar is it? Do we know? I wonder if there is
                alignment for this "unimportant cross-origin frame"
                concept, if we shouldn't specify that somehow.

                On 3/15/24 2:58 AM, Zheda Chen wrote:
                *Intent to Ship: Add JavaScript timer wake up
                alignment for unimportant cross-origin frames*

                Contact emails
                zheda...@intel.com, fdo...@chromium.org

                Specification
                
https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html
                
<https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html>

                Summary

                Align wake ups of JavaScript timers for unimportant
                cross-origin frames. Currently, DOM timers <32ms
                are all opt-out from AlignWakeUps [1] due to
                performance concerns. This is very conservative and
                actually some unimportant frames are eligible to
                use JS timer alignment. WebKit uses the policy to
                align DOM timer of non-interacted cross origin
                frames to 30ms. This feature adds JavaScript timer
                wake up alignment for unimportant frames on
                foreground pages. Unimportant frames means they are
                cross origin, visible but have non-large proportion
                of page’s visible area, and have no user
                interaction. The alignment interval is 32ms. [1]
                
https://chromium-review.googlesource.com/c/chromium/src/+/4589092
                
<https://chromium-review.googlesource.com/c/chromium/src/+/4589092>



                Blink component
                Blink>PerformanceAPIs>Timers
                
<https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EPerformanceAPIs%3ETimers>

                TAG review
                None

                TAG review status
                Not applicable

                Risks


                Interoperability and Compatibility

                "Other vendors already have interoperable
                implementation" Safari has a similar intervention.
                DOM timers of non-interacted cross origin frames
                are aligned to 30ms. In Firefox, all DOM timers
                (both same-origin and cross-origin) in foreground
                pages would be aligned to 16ms. "A mature
                specification in the relevant standards body" This
                intervention is spec-compliant. The spec mentions
                "the SetTimeout/SetInterval API does not guarantee
                that timers will run exactly on schedule. Delays
                due to CPU load, other tasks, etc, are to be
                expected. ". The wait length of time is
                implementation-defined, "which is intended to allow
                user agents to pad timeouts as needed to optimize
                the power usage of the device. "
                
https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html
                
<https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html>
                "A shared test suite for that specification" It
                isn't clear what should be tested specifically for
                this intervention since the spec allows waiting for
                an "implementation-defined" length.



                /Gecko/: Shipped/Shipping
                In Firefox, all DOM timers of frames (both same
                origin and cross origin) are aligned to 16ms

                /WebKit/: Shipped/Shipping
                In Safari, DOM timers of non-interacted cross
                origin frames are aligned to 30ms after reaching
                max nesting level (>=5)

                /Web developers/: No signals

                /Other signals/:

                WebView application risks

                Does this intent deprecate or change behavior of
                existing APIs, such that it has potentially high
                risk for Android WebView-based applications?

                None



                Debuggability

                None



                Will this feature be supported on all six Blink
                platforms (Windows, Mac, Linux, ChromeOS, Android,
                and Android WebView)?
                Yes

                Is this feature fully tested by web-platform-tests
                
<https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md>?
                Yes

                This intervention doesn't require changes to the
                spec. Timers are already covered by Web Platform Tests.



                Flag name on chrome://flags
                None

                Finch feature name
                ThrottleUnimportantFrameTimers

                Requires code in //chrome?
                False

                Tracking bug
                https://issues.chromium.org/issues/40942028
                <https://issues.chromium.org/issues/40942028>

                Launch bug
                https://issues.chromium.org/issues/40942028
                <https://issues.chromium.org/issues/40942028>

                Estimated milestones
                Shipping on desktop
                123
                DevTrial on desktop
                121
                Shipping on Android
                123
                DevTrial on Android
                121

                Anticipated spec changes

                Open questions about a feature may be a source of
                future web compat or interop issues. Please list
                open issues (e.g. links to known github issues in
                the project for the feature specification) whose
                resolution may introduce web compat/interop risk
                (e.g., changing to naming or structure of the API
                in a non-backward-compatible way).

                None

                Link to entry on the Chrome Platform Status
                https://chromestatus.com/feature/5106220399853568
                <https://chromestatus.com/feature/5106220399853568>

                Links to previous Intent discussions

                This intent message was generated by Chrome
                Platform Status <https://chromestatus.com/>.


                On Friday, March 15, 2024 at 6:24:06 AM UTC+8
                mike...@chromium.org wrote:

                    The privacy and security teams review all
                    intents, so requesting reviews and answering
                    the relevant questionnaires is the best path
                    forward.

                    Looking forward to the new I2S - thanks!

                    On 3/14/24 11:22 AM, Zheda Chen wrote:
                    More details are updated in ChromeStatus,
                    including interoperability and compatibility
                    risks, Safari/Firefox views, web platform
                    tests. It compares the behaviors across
                    different browser vendors.
                    https://chromestatus.com/feature/5106220399853568
                    <https://chromestatus.com/feature/5106220399853568>


                    Will send the updated "intent to ship" to this
                    thread later if it looks good.

                    AFAIK, I don't see potential privacy/security
                    issues, so can i set "security review status"
                    and "privacy review status" to "not
                    applicable"? Please let us know if you have
                    any concerns.

                    On Wednesday, March 13, 2024 at 10:00:35 AM
                    UTC+8 dom...@chromium.org wrote:

                        Can you fill out the interoperability and
                        compatibility risks section here? I don't
                        think standards position requests are
                        necessary, but saying how this behavior
                        might break existing sites that assume
                        Chromium's current behavior, and how this
                        new behavior compares to WebKit and Gecko,
                        would be helpful.

                        Also, can you request the
                        privacy/security/enterprise/debuggability/testing
                        gates on Chrome Status?

                        On Tue, Mar 12, 2024 at 10:12 PM Zheda
                        Chen <zheda...@intel.com> wrote:

                            Okay I update the process stage in
                            Chrome Platform Status, and paste the
                            newly-generated Intent above. Please
                            take a look.

                            https://chromestatus.com/feature/5106220399853568
                            <https://chromestatus.com/feature/5106220399853568>

-- You received this message because you are subscribed to
            the Google Groups "blink-dev" group.
            To unsubscribe from this group and stop receiving emails
            from it, send an email to
            blink-dev+unsubscr...@chromium.org
            <mailto:blink-dev+unsubscr...@chromium.org>.
            To view this discussion on the web visit
            
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/7b8b9b84-2285-4fa9-a80d-c5fc0c1c0c41n%40chromium.org
            
<https://groups.google.com/a/chromium.org/d/msgid/blink-dev/7b8b9b84-2285-4fa9-a80d-c5fc0c1c0c41n%40chromium.org?utm_medium=email&utm_source=footer>.


--
You received this message because you are subscribed to the Google Groups 
"blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit 
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/1a56a1fd-2fdf-413a-85b4-4a879de2f816%40chromium.org.

Reply via email to