Keep ImageLoader alive when decode is pending.

JS may invoke Image.decode without capturing the Image object. If GC kicks in, ImageLoader will be destroyed as no one owns it, which leads to unresolved/unrejected Promise. The callback of ChromeClient::RequestDecode should take a persistent reference instead of weak reference to avoid ImageLoader GC. BUG=881060 TEST=LayoutTest image-decode-promise-when-gc.html Change-Id: Ief9c7956ddd0df81ea1f1643eb6f6c7bac946a7c Reviewed-on: https://chromium-review.googlesource.com/1205798Reviewed-by: Nate Chapin <japhet@chromium.org> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: vmpstr <vmpstr@chromium.org> Commit-Queue: Yuchen Liu <yucliu@chromium.org> Cr-Commit-Position: refs/heads/master@{#589687}

Keep ImageLoader alive when decode is pending.
JS may invoke Image.decode without capturing the Image object. If GC kicks in, ImageLoader will be destroyed as no one owns it, which leads to unresolved/unrejected Promise. The callback of ChromeClient::RequestDecode should take a persistent reference instead of weak reference to avoid ImageLoader GC. BUG=881060 TEST=LayoutTest image-decode-promise-when-gc.html Change-Id: Ief9c7956ddd0df81ea1f1643eb6f6c7bac946a7c Reviewed-on: https://chromium-review.googlesource.com/1205798Reviewed-by: Nate Chapin <japhet@chromium.org> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: vmpstr <vmpstr@chromium.org> Commit-Queue: Yuchen Liu <yucliu@chromium.org> Cr-Commit-Position: refs/heads/master@{#589687}
fc143229 · Yuchen Liu · Commit Bot · 2c6f64dd · fc143229 · fc143229
Commit fc143229 authored Sep 07, 2018 by Yuchen Liu Committed by Commit Bot Sep 07, 2018
3 changed files
--- a/third_party/WebKit/LayoutTests/fast/loader/image-decode-promise-when-gc-expected.txt
+++ b/third_party/WebKit/LayoutTests/fast/loader/image-decode-promise-when-gc-expected.txt
+Image object may not be captured by the promise of Image.decode.
+If GC kicks in before promise is resolved/rejected, Image object should be kept alive.
+PASS; no crash.
--- a/third_party/WebKit/LayoutTests/fast/loader/image-decode-promise-when-gc.html
+++ b/third_party/WebKit/LayoutTests/fast/loader/image-decode-promise-when-gc.html
+<!DOCTYPE html>
+<body>
+Image object may not be captured by the promise of Image.decode.</br>
+If GC kicks in before promise is resolved/rejected, Image object should be kept alive.</br>
+</body>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+let decode_promises = [];
+
+for (var i = 0; i < 5; i++) {
+    let image = new Image();
+    image.src = '../../images/resources/apng00.png';
+    decode_promises.push(image.decode());
+}
+
+Promise.all(decode_promises).then(() => {
+    document.body.innerHTML += 'PASS; no crash.';
+
+    if (window.testRunner)
+        testRunner.notifyDone();
+});
+
+// Aggressive GC to trigger "GC when decode callback is pending".
+if (window.GCController) {
+    setInterval(() => {
+        GCController.collectAll();
+    }, 1);
+}
+</script>
--- a/third_party/blink/renderer/core/loader/image_loader.cc
+++ b/third_party/blink/renderer/core/loader/image_loader.cc
@@ -190,10 +190,14 @@ void ImageLoader::DispatchDecodeRequestsIfComplete() {
    if (request->state() != DecodeRequest::kPendingLoad)
      continue;
    Image* image = GetContent()->GetImage();
+
+    // ImageLoader should be kept alive when decode is still pending. JS may
+    // invoke 'decode' without capturing the Image object. If GC kicks in,
+    // ImageLoader will be destroyed, leading to unresolved/unrejected Promise.
    frame->GetChromeClient().RequestDecode(
        frame, image->PaintImageForCurrentFrame(),
        WTF::Bind(&ImageLoader::DecodeRequestFinished,
-                  WrapCrossThreadWeakPersistent(this), request->request_id()));
+                  WrapCrossThreadPersistent(this), request->request_id()));
    request->NotifyDecodeDispatched();
  }
 }