Bug 163878

<rdar://problem/28910901>

Created attachment 292580 [details] Fixes the bug

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.

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.

(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.

Created attachment 292718 [details] Patch for landing

Comment on attachment 292718 [details] Patch for landing Clearing flags on attachment: 292718 Committed r207810: <http://trac.webkit.org/changeset/207810>

All reviewed patches have been landed. Closing bug.