Make CSSFontFace::setLoadStatus post a task

third_party/WebKit/Source/core/css/FontFace.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 36 matching lines @@
 #include "core/css/RemoteFontFaceSource.h"
 #include "core/css/StylePropertySet.h"
 #include "core/css/StyleRule.h"
 #include "core/css/parser/CSSParser.h"
 #include "core/dom/DOMArrayBuffer.h"
 #include "core/dom/DOMArrayBufferView.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/StyleEngine.h"
+#include "core/dom/TaskRunnerHelper.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
 #include "core/frame/UseCounter.h"
 #include "platform/FontFamilyNames.h"
 #include "platform/Histogram.h"
 #include "platform/SharedBuffer.h"
+#include "platform/WebTaskRunner.h"
 
 namespace blink {
 
 static const CSSValue* parseCSSValue(const Document* document,
                                      const String& value,
                                      CSSPropertyID propertyID) {
   CSSParserContext context(*document, UseCounter::getFrom(document));
   return CSSParser::parseFontFaceDescriptor(propertyID, value, context);
 }
 
@@ skipping 272 matching lines @@
   }
   return emptyString();
 }
 
 void FontFace::setLoadStatus(LoadStatusType status) {
   m_status = status;
   ASSERT(m_status != Error || m_error);
 
   if (m_status == Loaded || m_status == Error) {
     if (m_loadedProperty) {
-      if (m_status == Loaded)
-        m_loadedProperty->resolve(this);
-      else
+      if (m_status == Loaded) {
+        TaskRunnerHelper::get(TaskType::DOMManipulation, getExecutionContext())
+            ->postTask(BLINK_FROM_HERE,
+                       WTF::bind(&LoadedProperty::resolve<FontFace*>,
+                                 wrapPersistent(m_loadedProperty.get()),
+                                 wrapPersistent(this)));
+      } else
         m_loadedProperty->reject(m_error.get());

[Kunihiko Sakamoto, 2017/01/10 06:02:12]

Need to post a task for rejection too?

[adithyas, 2017/01/10 17:52:46]

Rejection is always asynchronous so we don't have to post a task.

[Kunihiko Sakamoto, 2017/01/11 05:25:45]

Ah interesting, didn't know that.

[adithyas, 2017/01/11 17:59:39]

Technically, resolution is also always asynchronous (the callback is executed in
a separate microtask). But resolving promises involves accessing the 'then'
property of the resolved value, so a getter for 'then' could be executed
synchronously.

     }
 
     HeapVector<Member<LoadFontCallback>> callbacks;
     m_callbacks.swap(callbacks);
     for (size_t i = 0; i < callbacks.size(); ++i) {
       if (m_status == Loaded)
         callbacks[i]->notifyLoaded(this);

[Kunihiko Sakamoto, 2017/01/10 06:02:12]

This may resolve a promise (that is created by FontFaceSet.load()).

So I think we should delay the body of
 if (m_status == Loaded || m_status == Error) { ... }
entirely.

[adithyas, 2017/01/10 17:52:46]

FontFaceSet uses an AsyncMethodRunner to resolve the promise/fire a done event,
so that's already asynchronous.

[Kunihiko Sakamoto, 2017/01/11 05:25:45]

FontFaceSet::notifyLoaded() is OK, but LoadFontPromiseResolver::notifyLoaded()
resolves m_resolver synchronously.

[adithyas, 2017/01/11 17:59:39]

Ahh okay, I've changed it to run the callbacks in a task. 

       else
         callbacks[i]->notifyError(this);
     }
   }
 }
 
 void FontFace::setError(DOMException* error) {
   if (!m_error)
     m_error = error ? error : DOMException::create(NetworkError);
   setLoadStatus(Error);
@@ skipping 264 matching lines @@
 
 bool FontFace::hadBlankText() const {
   return m_cssFontFace->hadBlankText();
 }
 
 bool FontFace::hasPendingActivity() const {
   return m_status == Loading && getExecutionContext();
 }
 
 }  // namespace blink