WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
163878
Custom elements reactions should have a queue per element
https://bugs.webkit.org/show_bug.cgi?id=163878
Summary
Custom elements reactions should have a queue per element
Ryosuke Niwa
Reported
2016-10-23 20:50:31 PDT
Update our reactions queue' semantics to match the latest specifications.
Attachments
Fixes the bug
(39.44 KB, patch)
2016-10-23 21:48 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Patch for landing
(39.25 KB, patch)
2016-10-24 22:14 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2016-10-23 21:46:49 PDT
<
rdar://problem/28910901
>
Ryosuke Niwa
Comment 2
2016-10-23 21:48:08 PDT
Created
attachment 292580
[details]
Fixes the bug
WebKit Commit Bot
Comment 3
2016-10-23 21:49:58 PDT
Attachment 292580
[details]
did not pass style-queue: ERROR: Source/WebCore/dom/CustomElementReactionQueue.cpp:250: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Antti Koivisto
Comment 4
2016-10-24 02:38:52 PDT
Comment on
attachment 292580
[details]
Fixes the bug View in context:
https://bugs.webkit.org/attachment.cgi?id=292580&action=review
> Source/WebCore/dom/CustomElementReactionQueue.cpp:254 > +class BackupElementQueueMicrotask final : public Microtask { > + WTF_MAKE_FAST_ALLOCATED; > +private: > + Result run() final > + { > + CustomElementReactionStack::processBackupQueue(); > + return Result::Done; > + } > +};
Someone should modernize Microtask to take lambdas rather than require subclassing.
> Source/WebCore/dom/CustomElementReactionQueue.cpp:264 > +CustomElementReactionStack::ElementQueue& CustomElementReactionStack::ensureBackupQueue() > +{ > + auto& queue = backupElementQueue(); > + if (queue) > + return *queue; // If the processing the backup element queue flag is set, abort this algorithm. > + queue = std::make_unique<ElementQueue>(); // Set the processing the backup element queue flag. > + MicrotaskQueue::mainThreadQueue().append(std::make_unique<BackupElementQueueMicrotask>()); > + return *queue; > +}
I wonder if this would be easier to follow if there was an actual "processing the backup element queue" boolean flag instead of using the existence of backupElementQueue object. backupElementQueue could just stay alive like normal global NeverDestroyed.
Ryosuke Niwa
Comment 5
2016-10-24 22:08:30 PDT
(In reply to
comment #4
)
> Comment on
attachment 292580
[details]
> Fixes the bug > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=292580&action=review
> > > Source/WebCore/dom/CustomElementReactionQueue.cpp:254 > > +class BackupElementQueueMicrotask final : public Microtask { > > + WTF_MAKE_FAST_ALLOCATED; > > +private: > > + Result run() final > > + { > > + CustomElementReactionStack::processBackupQueue(); > > + return Result::Done; > > + } > > +}; > > Someone should modernize Microtask to take lambdas rather than require > subclassing.
Yeah, we should do that.
> > Source/WebCore/dom/CustomElementReactionQueue.cpp:264 > > +CustomElementReactionStack::ElementQueue& CustomElementReactionStack::ensureBackupQueue() > > +{ > > + auto& queue = backupElementQueue(); > > + if (queue) > > + return *queue; // If the processing the backup element queue flag is set, abort this algorithm. > > + queue = std::make_unique<ElementQueue>(); // Set the processing the backup element queue flag. > > + MicrotaskQueue::mainThreadQueue().append(std::make_unique<BackupElementQueueMicrotask>()); > > + return *queue; > > +} > > I wonder if this would be easier to follow if there was an actual > "processing the backup element queue" boolean flag instead of using the > existence of backupElementQueue object. backupElementQueue could just stay > alive like normal global NeverDestroyed.
Done that.
Ryosuke Niwa
Comment 6
2016-10-24 22:14:13 PDT
Created
attachment 292718
[details]
Patch for landing
WebKit Commit Bot
Comment 7
2016-10-24 23:21:02 PDT
Comment on
attachment 292718
[details]
Patch for landing Clearing flags on attachment: 292718 Committed
r207810
: <
http://trac.webkit.org/changeset/207810
>
WebKit Commit Bot
Comment 8
2016-10-24 23:21:10 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug