Make ImageObserver::AsyncLoadCompleted() called asynchronously

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

 /*
  * Copyright (C) 2006 Eric Seidel <eric@webkit.org>
  * Copyright (C) 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) Research In Motion Limited 2011. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
@@ skipping 12 matching lines @@
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "core/svg/graphics/SVGImage.h"
 
 #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"
 #include "core/frame/LocalFrameClient.h"
 #include "core/frame/Settings.h"
 #include "core/layout/LayoutView.h"
 #include "core/layout/svg/LayoutSVGRoot.h"
 #include "core/loader/FrameLoadRequest.h"
 #include "core/paint/FloatClipRecorder.h"
 #include "core/paint/TransformRecorder.h"
@@ skipping 590 matching lines @@
 }
 
 void SVGImage::LoadCompleted() {
   switch (load_state_) {
     case kInDataChanged:
       load_state_ = kLoadCompleted;
       break;
 
     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:
     case kLoadCompleted:
       CHECK(false);
       break;
   }
 }
 
 Image::SizeAvailability SVGImage::DataChanged(bool all_data_received) {
@@ skipping 104 matching lines @@
 
   NOTREACHED();
   return kSizeAvailable;
 }
 
 String SVGImage::FilenameExtension() const {
   return "svg";
 }
 
 }  // namespace blink