Revert of Fix ImageLoader::m_hasPendingLoadEvent/m_imageComplete in...

Revert of Fix ImageLoader::m_hasPendingLoadEvent/m_imageComplete in ImageDocument (patchset #4 id:60001 of https://codereview.chromium.org/2208073004/ )

Reason for revert:
Reverting since http/tests/images/png-partial-load-as-document.html consistently leaks on WebKit Linux Leak and LeakExpectations says "Sheriff is expected to revert culprit CLs instead of suppressing the leaks"

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=http%2Ftests%2Fimages%2Fpng-partial-load-as-document.html&testType=webkit_tests

Original issue's description:
> 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
>
> Committed: https://crrev.com/576c7a048bfc30faf510eeda07f303fa9dd12898
> Cr-Commit-Position: refs/heads/master@{#414008}

TBR=japhet@chromium.org,yhirano@chromium.org,hiroshige@chromium.org
NOTRY=true
NOTREECHECKS=true
BUG=632495

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

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,38 +217,6 @@ 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)
@@ -344,7 +312,33 @@ void ImageLoader::doUpdateFromElement(BypassMainWorldBehavior bypassBehavior, Up
     if (updateBehavior == UpdateSizeChanged && m_element->layoutObject() && m_element->layoutObject()->isImage() && newImage == oldImage) {
         toLayoutImage(m_element->layoutObject())->intrinsicSizeChanged();
     } else {
-        setImagePending(newImage);
+        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);
+        }
     }
 
     if (LayoutImageResource* imageResource = layoutImageResource())
@@ -372,11 +366,8 @@ 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) {
-        setImagePending(ImageResource::create(imageSourceToKURL(m_element->imageSourceURL())));
+        setImage(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,12 +128,7 @@ 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);