Make ImageObserver::AsyncLoadCompleted() called asynchronously

third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp

Index: third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
diff --git a/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp b/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
index 8c34ea90b7adca781799ab40528b5f1a96055a91..5af059e5e22570913c9fcf0067604eace2dbdf95 100644
--- a/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
+++ b/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
@@ -30,6 +30,7 @@
 #include "core/animation/DocumentAnimations.h"
 #include "core/animation/DocumentTimeline.h"
 #include "core/dom/NodeTraversal.h"
+#include "core/dom/TaskRunnerHelper.h"
 #include "core/dom/shadow/FlatTreeTraversal.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
@@ -640,8 +641,15 @@ void SVGImage::LoadCompleted() {
 
     case kWaitingForAsyncLoadCompletion:
       load_state_ = kLoadCompleted;
-      if (GetImageObserver())
-        GetImageObserver()->AsyncLoadCompleted(this);
+      if (GetImageObserver()) {
+        TaskRunnerHelper::Get(TaskType::kDOMManipulation,

[fs, 2017/05/19 09:06:49]

Since this is a task posted on a task runner belonging to the frame of the
SVGImage (not the client/initiator of the image load), I'm not sure how this
should be interpreted. My feeling is that this task shouldn't go on the DOM
Manipulation task source though - maybe kUnspecedLoading would be more
appropriate?
Also related, but I can't help feeling that maybe this should rather be posting
the task to itself and check and notify the observer. I.e:

load_state_ = kLoadCompleted;
...->PostTask(..., WTF::Bind(&SVGImage::NotifyAsyncLoadCompleted,
RefPtr<SVGImage>(this)));

void SVGImage::NotifyAsyncLoadCompleted() {
  if (ImageObserver* observer = GetImageObserver())
    observer->AsyncLoadCompleted(this);
}

(Not overly enthused by the RefPtr, but...)

This would compartmentalize things a bit better IMHO. WDYT?

[hiroshige, 2017/05/19 20:28:52]

This is because
- The frame etc. of the "client/initiator of the image load" is
  not accecible from SVGImage here, and
- This is a kind of half-deferring the
  DispatchDidHandleOnloadEvents() callback tied to the Document of
  |page_|.
Done, but I'm also not sure.

haraken@, any suggestions about TaskType and TaskRunner to use?
Looks nice, done.

+                              ToLocalFrame(page_->MainFrame())->GetDocument())

[kouhei (in TOK), 2017/05/19 05:20:54]

->GetDocument() not needed. TaskRunnerHelper::Get also takes LocalFrame.

[hiroshige, 2017/05/19 20:28:52]

Done.

+            ->PostTask(
+                BLINK_FROM_HERE,
+                WTF::Bind(&ImageObserver::AsyncLoadCompleted,
+                          WeakPersistent<ImageObserver>(GetImageObserver()),

[yhirano, 2017/05/19 06:55:29]

Why weak?

[hiroshige, 2017/05/19 20:28:52]

I think Persistent will also work well here.
I made this WeakPersistent because I think AsyncLoadCompleted() is no longer
needed if ImageObserver is destructed before invocation.

+                          RefPtr<SVGImage>(this)));
+      }
       break;
 
     case kDataChangedNotStarted: