Bug 162726 - [Modern Media Controls] scheduler for layout nodes
Summary: [Modern Media Controls] scheduler for layout nodes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-09-29 07:04 PDT by Antoine Quint
Modified: 2016-09-29 13:04 PDT (History)
3 users (show)

See Also:


Attachments
Patch (8.05 KB, patch)
2016-09-29 07:08 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (8.12 KB, patch)
2016-09-29 07:11 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (8.46 KB, patch)
2016-09-29 12:05 PDT, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2016-09-29 07:04:28 PDT
[Modern Media Controls] scheduler for layout nodes
Comment 1 Antoine Quint 2016-09-29 07:08:09 PDT
Created attachment 290202 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2016-09-29 07:09:20 PDT
<rdar://problem/28543043>
Comment 3 Antoine Quint 2016-09-29 07:11:58 PDT
Created attachment 290203 [details]
Patch
Comment 4 Dean Jackson 2016-09-29 09:56:25 PDT
Comment on attachment 290203 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290203&action=review

> Source/WebCore/ChangeLog:13
> +        For the work on the modern media controls we will be using a tree of LayoutNode objects that will
> +        commit to the DOM in coordinated `requestAnimationFrame()` calls to ensure all layouts are done
> +        in an efficient and coordinated manner. As a preamble, we introduce a `scheduler` singleton which
> +        provides a single public method `scheduleLayout(callback)` to queue callbacks to be fired in the
> +        next `requestAnimationFrame()` callback.

Why are you using those weird quotes?

> LayoutTests/media/modern-media-controls/scheduler/not-reentrant.html:35
> +
> +    println(`scheduler.scheduleLayout(layout)`);

Add a comment here saying that this is the important part of the test - scheduling layout from within layout.

> LayoutTests/media/modern-media-controls/scheduler/not-reentrant.html:44
> +    output.innerHTML += `<div>${line}</div>`;

Yuck. Guess it's fine for this small test.
Comment 5 Antoine Quint 2016-09-29 10:28:26 PDT
(In reply to comment #4)
> Comment on attachment 290203 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290203&action=review
> 
> > Source/WebCore/ChangeLog:13
> > +        For the work on the modern media controls we will be using a tree of LayoutNode objects that will
> > +        commit to the DOM in coordinated `requestAnimationFrame()` calls to ensure all layouts are done
> > +        in an efficient and coordinated manner. As a preamble, we introduce a `scheduler` singleton which
> > +        provides a single public method `scheduleLayout(callback)` to queue callbacks to be fired in the
> > +        next `requestAnimationFrame()` callback.
> 
> Why are you using those weird quotes?

Old habits from Markdown. This is how you codify code in Markdown, or at least the GitHub-flavored version.

> > LayoutTests/media/modern-media-controls/scheduler/not-reentrant.html:35
> > +
> > +    println(`scheduler.scheduleLayout(layout)`);
> 
> Add a comment here saying that this is the important part of the test -
> scheduling layout from within layout.

Good point, will do.

> > LayoutTests/media/modern-media-controls/scheduler/not-reentrant.html:44
> > +    output.innerHTML += `<div>${line}</div>`;
> 
> Yuck. Guess it's fine for this small test.

I was looking for some existing test setup to do this type of stuff. Is there something better I should be (re)using?
Comment 6 Antoine Quint 2016-09-29 12:05:09 PDT
Created attachment 290229 [details]
Patch
Comment 7 WebKit Commit Bot 2016-09-29 13:04:48 PDT
Comment on attachment 290229 [details]
Patch

Clearing flags on attachment: 290229

Committed r206606: <http://trac.webkit.org/changeset/206606>
Comment 8 WebKit Commit Bot 2016-09-29 13:04:51 PDT
All reviewed patches have been landed.  Closing bug.