Fix ImageLoader::m_hasPendingLoadEvent/m_imageComplete in ImageDocument

Case 1: When a image is loaded as a subresource, loading is stared by
ImageResource::fetch() in ImageLoader::doUpdateFromElement().
Case 2: When a image is loaded as a main document, the start of the loading is
emulated by setImage() in ImageLoader::updateFromElement() and
ImageDocumentParser will do the subsequent loading steps.

However in Case 2, |m_hasPendingLoadEvent| is set to false
(in setImageWithoutConsideringPendingLoadEvent()), causing |imageStillLoading|
to be false and ensureFallbackContent() to be called in
HTMLImageElement::selectSourceURL(), and thus a box indicating a broken image
is displayed during loading instead of the image displayed progressively.
This is regression since https://codereview.chromium.org/1879793003.

This CL makes the behavior of Case 1 and 2 consistent, by moving what is done
in Case 1 to setImagePending(), and makes Case 1 and 2 both to call
setImagePending().

This CL also adds a layout test to confirm that an image is displayed
progressively when loaded as a main document.

BUG=632495

Review-Url: https://codereview.chromium.org/2208073004
Cr-Commit-Position: refs/heads/master@{#414008}

third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document-expected.txt

+

third_party/WebKit/LayoutTests/http/tests/images/png-partial-load-as-document.html

+<iframe style="background-color: blue; width: 320px; height: 240px;"></iframe>
+<!--
+Tests that when a progressive png is loaded as document, the image is shown
+progressively during loading.
+The result image should show the dice image partially.
+-->
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsTextWithPixelResults();
+    testRunner.waitUntilDone();
+}
+
+function loadAndStall()
+{
+    return "http://127.0.0.1:8000/resources/load-and-stall.php";
+}
+
+function pngImage()
+{
+    return "?name=../../../fast/images/resources/dice.png&mimeType=image%2Fpng";
+}
+
+function testDone()
+{
+    if (window.testRunner) {
+        window.stop();
+        testRunner.notifyDone();
+    }
+}
+
+function runTest()
+{
+    document.querySelector("iframe").src = loadAndStall() + pngImage() + "&stallAt=45057&stallFor=60";
+    setTimeout(testDone, 500);
+}
+
+window.onload = function() {
+    setTimeout(runTest, 0);
+}
+</script>

third_party/WebKit/Source/core/loader/ImageLoader.cpp

@@ -217,6 +217,38 @@ void ImageLoader::setImageWithoutConsideringPendingLoadEvent(ImageResource* newI
         imageResource->resetAnimation();
 }
 
+void ImageLoader::setImagePending(ImageResource* newImage)
+{
+    ImageResource* oldImage = m_image.get();
+    if (m_hasPendingLoadEvent) {
+        loadEventSender().cancelEvent(this);
+        m_hasPendingLoadEvent = false;
+    }
+
+    // Cancel error events that belong to the previous load, which is now cancelled by changing the src attribute.
+    // If newImage is null and m_hasPendingErrorEvent is true, we know the error event has been just posted by
+    // this load and we should not cancel the event.
+    // FIXME: If both previous load and this one got blocked with an error, we can receive one error event instead of two.
+    if (m_hasPendingErrorEvent && newImage) {
+        errorEventSender().cancelEvent(this);
+        m_hasPendingErrorEvent = false;
+    }
+
+    m_image = newImage;
+    m_hasPendingLoadEvent = newImage;
+    m_imageComplete = !newImage;
+
+    updateLayoutObject();
+    // If newImage exists and is cached, addObserver() will result in the load event
+    // being queued to fire. Ensure this happens after beforeload is dispatched.
+    if (newImage) {
+        newImage->addObserver(this);
+    }
+    if (oldImage) {
+        oldImage->removeObserver(this);
+    }
+}
+
 static void configureRequest(FetchRequest& request, ImageLoader::BypassMainWorldBehavior bypassBehavior, Element& element, const ClientHintsPreferences& clientHintsPreferences)
 {
     if (bypassBehavior == ImageLoader::BypassMainWorldCSP)
@@ -312,33 +344,7 @@ void ImageLoader::doUpdateFromElement(BypassMainWorldBehavior bypassBehavior, Up
     if (updateBehavior == UpdateSizeChanged && m_element->layoutObject() && m_element->layoutObject()->isImage() && newImage == oldImage) {
         toLayoutImage(m_element->layoutObject())->intrinsicSizeChanged();
     } else {
-        if (m_hasPendingLoadEvent) {
-            loadEventSender().cancelEvent(this);
-            m_hasPendingLoadEvent = false;
-        }
-
-        // Cancel error events that belong to the previous load, which is now cancelled by changing the src attribute.
-        // If newImage is null and m_hasPendingErrorEvent is true, we know the error event has been just posted by
-        // this load and we should not cancel the event.
-        // FIXME: If both previous load and this one got blocked with an error, we can receive one error event instead of two.
-        if (m_hasPendingErrorEvent && newImage) {
-            errorEventSender().cancelEvent(this);
-            m_hasPendingErrorEvent = false;
-        }
-
-        m_image = newImage;
-        m_hasPendingLoadEvent = newImage;
-        m_imageComplete = !newImage;
-
-        updateLayoutObject();
-        // If newImage exists and is cached, addObserver() will result in the load event
-        // being queued to fire. Ensure this happens after beforeload is dispatched.
-        if (newImage) {
-            newImage->addObserver(this);
-        }
-        if (oldImage) {
-            oldImage->removeObserver(this);
-        }
+        setImagePending(newImage);
     }
 
     if (LayoutImageResource* imageResource = layoutImageResource())
@@ -366,8 +372,11 @@ void ImageLoader::updateFromElement(UpdateFromElementBehavior updateBehavior, Re
     // funneling the main resource bytes into m_image, so just create an ImageResource
     // to be populated later.
     if (m_loadingImageDocument && updateBehavior != UpdateForcedReload) {
-        setImage(ImageResource::create(imageSourceToKURL(m_element->imageSourceURL())));
+        setImagePending(ImageResource::create(imageSourceToKURL(m_element->imageSourceURL())));
         m_image->setStatus(Resource::Pending);
+        // Only consider updating the protection ref-count of the Element immediately before returning
+        // from this function as doing so might result in the destruction of this ImageLoader.
+        updatedHasPendingEvent();
         return;
     }
 

third_party/WebKit/Source/core/loader/ImageLoader.h

@@ -128,7 +128,12 @@ private:
     LayoutImageResource* layoutImageResource();
     void updateLayoutObject();
 
+    // Sets |m_image| and flags as loading is completed.
     void setImageWithoutConsideringPendingLoadEvent(ImageResource*);
+    // Sets |m_image| and flags as loading is pending/not completed
+    // if |newImage| is non-null.
+    void setImagePending(ImageResource* /* newImage */);
+
     void clearFailedLoadURL();
     void dispatchErrorEvent();
     void crossSiteOrCSPViolationOccurred(AtomicString);