Change the semantics of ImageLoader::has_pending_load_event_

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2009, 2010 Apple Inc. All rights
  * reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
@@ skipping 145 matching lines @@
 
 ImageLoader::ImageLoader(Element* element)
     : element_(element),
       deref_element_timer_(this, &ImageLoader::TimerFired),
       has_pending_load_event_(false),
       has_pending_error_event_(false),
       image_complete_(true),
       loading_image_document_(false),
       element_is_protected_(false),
       suppress_error_events_(false) {
   RESOURCE_LOADING_DVLOG(1) << "new ImageLoader " << this;

[hiroshige, 2017/05/05 19:58:31]

Previous: has_pending_load_event_ == false.

Now: (image_ && !image_complete_) || has_pending_load_event_ == false:
  has_pending_load_event_ == false
  image_complete_ == true
  image_ == null

 }
 
 ImageLoader::~ImageLoader() {}
 
 void ImageLoader::Dispose() {
   RESOURCE_LOADING_DVLOG(1)
       << "~ImageLoader " << this
       << "; m_hasPendingLoadEvent=" << has_pending_load_event_

[hiroshige, 2017/05/05 19:58:31]

I left logging of has_pending_load_event_ as is. ditto below.

       << ", m_hasPendingErrorEvent=" << has_pending_error_event_;
 
   if (image_) {
     image_->RemoveObserver(this);
     image_ = nullptr;
     delay_until_image_notify_finished_ = nullptr;
   }
 }
 
 DEFINE_TRACE(ImageLoader) {
@@ skipping 11 matching lines @@
   UpdatedHasPendingEvent();
 }
 
 void ImageLoader::SetImageWithoutConsideringPendingLoadEvent(
     ImageResourceContent* new_image) {
   DCHECK(failed_load_url_.IsEmpty());
   ImageResourceContent* old_image = image_.Get();
   if (new_image != old_image) {
     image_ = new_image;
     delay_until_image_notify_finished_ = nullptr;
     if (has_pending_load_event_) {

[hiroshige, 2017/05/05 19:58:31]

Here we don't check "(image_ && !image_complete_) || has_pending_load_event_"
because, if has_pending_load_event_ == false,
Lines 207--208 should be no-op.

       LoadEventSender().CancelEvent(this);
       has_pending_load_event_ = false;
     }
     if (has_pending_error_event_) {
       ErrorEventSender().CancelEvent(this);
       has_pending_error_event_ = false;
     }
     image_complete_ = true;
     if (new_image) {
       new_image->AddObserver(this);
     }
     if (old_image) {
       old_image->RemoveObserver(this);
     }

[hiroshige, 2017/05/05 19:58:31]

Previous: has_pending_load_event_ == false.

Now: (image_ && !image_complete_) || has_pending_load_event_ == false:
  has_pending_load_event_ == false
  image_complete_ == true

   }
 
   if (LayoutImageResource* image_resource = GetLayoutImageResource())
     image_resource->ResetAnimation();
 }
 
 static void ConfigureRequest(
     FetchParameters& params,
     ImageLoader::BypassMainWorldBehavior bypass_behavior,
     Element& element,
@@ skipping 104 matching lines @@
       DispatchErrorEvent();
     }
     NoImageResourceToLoad();
   }
 
   ImageResourceContent* old_image = image_.Get();
   if (update_behavior == kUpdateSizeChanged && element_->GetLayoutObject() &&
       element_->GetLayoutObject()->IsImage() && new_image == old_image) {
     ToLayoutImage(element_->GetLayoutObject())->IntrinsicSizeChanged();
   } else {
     if (has_pending_load_event_) {

[hiroshige, 2017/05/05 19:58:31]

Ditto as Line 206.

       LoadEventSender().CancelEvent(this);
       has_pending_load_event_ = 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 (has_pending_error_event_ && new_image) {
       ErrorEventSender().CancelEvent(this);
       has_pending_error_event_ = false;
     }
 
     image_ = new_image;
-    has_pending_load_event_ = new_image;
     image_complete_ = !new_image;
     delay_until_image_notify_finished_ = nullptr;
 

[hiroshige, 2017/05/05 19:58:31]

Previous: has_pending_load_event_ == true iff new_image is non-null.

Now: if new_image is non-null,
  (image_ && !image_complete_) || has_pending_load_event_ == true:
    has_pending_load_event_ == false
    image_complete_ == false
    image_ != null

  otherwise,
  (image_ && !image_complete_) || has_pending_load_event_ == false:
    has_pending_load_event_ == false
    image_complete_ == true
    image == null

     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 (new_image) {
       new_image->AddObserver(this);
     }
     if (old_image) {
       old_image->RemoveObserver(this);
     }
@@ skipping 132 matching lines @@
   // Update ImageAnimationPolicy for m_image.
   if (image_)
     image_->UpdateImageAnimationPolicy();
 
   UpdateLayoutObject();
 
   if (image_ && image_->GetImage() && image_->GetImage()->IsSVGImage())
     ToSVGImage(image_->GetImage())
         ->UpdateUseCounters(GetElement()->GetDocument());
 
-  if (loading_image_document_) {
-    CHECK(!has_pending_load_event_);
+  if (loading_image_document_)
     return;
-  }
-
-  CHECK(has_pending_load_event_);

[hiroshige, 2017/05/05 19:58:31]

Previous: We assert has_pending_load_event_ == true.

Now: This means
     (image_ && !image_complete_) || has_pending_load_event_ == true,
     at the beginning of ImageNotifyFinished().
     image_ is non-null (as ImageNotifyFinished() is called),
     image_complete_ is false (asserted at Line 502)
     therefore this assert always holds and I removed this CHECK().
     (actually has_pending_load_event_ is expected to be false)

 
   if (resource->ErrorOccurred()) {
     LoadEventSender().CancelEvent(this);
     has_pending_load_event_ = false;

[hiroshige, 2017/05/05 19:58:31]

Because |image_complete_| is true here,
"(image_ && !image_complete_) || has_pending_load_event_"
is equivalent to has_pending_load_event_, so we just set it to false.

 
     if (resource->GetResourceError().IsAccessCheck()) {
       CrossSiteOrCSPViolationOccurred(
           AtomicString(resource->GetResourceError().FailingURL()));
     }
 
     // The error event should not fire if the image data update is a result of
     // environment change.
     // https://html.spec.whatwg.org/multipage/embedded-content.html#the-img-element:the-img-element-55
     if (!suppress_error_events_)
       DispatchErrorEvent();
 
     // 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;
   }
+  has_pending_load_event_ = true;
   LoadEventSender().DispatchEventSoon(this);
 }
 
 LayoutImageResource* ImageLoader::GetLayoutImageResource() {
   LayoutObject* layout_object = element_->GetLayoutObject();
 
   if (!layout_object)
     return 0;
 
   // We don't return style generated image because it doesn't belong to the
@@ skipping 26 matching lines @@
 }
 
 void ImageLoader::UpdatedHasPendingEvent() {
   // If an Element that does image loading is removed from the DOM the
   // load/error event for the image is still observable. As long as the
   // ImageLoader is actively loading, the Element itself needs to be ref'ed to
   // keep it from being destroyed by DOM manipulation or garbage collection. If
   // such an Element wishes for the load to stop when removed from the DOM it
   // needs to stop the ImageLoader explicitly.
   bool was_protected = element_is_protected_;
-  element_is_protected_ = has_pending_load_event_ || has_pending_error_event_;
+  element_is_protected_ = (image_ && !image_complete_) ||
+                          has_pending_load_event_ || has_pending_error_event_;
   if (was_protected == element_is_protected_)
     return;
 
   if (element_is_protected_) {
     if (deref_element_timer_.IsActive())
       deref_element_timer_.Stop();
     else
       keep_alive_ = element_;
   } else {
     DCHECK(!deref_element_timer_.IsActive());
@@ skipping 15 matching lines @@
   if (event_type == EventTypeNames::error)
     DispatchPendingErrorEvent();
 }
 
 void ImageLoader::DispatchPendingLoadEvent() {
   CHECK(has_pending_load_event_);
   if (!image_)
     return;
   CHECK(image_complete_);
   has_pending_load_event_ = false;
+  CHECK(image_complete_);
   if (GetElement()->GetDocument().GetFrame())
     DispatchLoadEvent();
 
   // 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();
 }
 
 void ImageLoader::DispatchPendingErrorEvent() {
@@ skipping 33 matching lines @@
   }
   if (delay_until_image_notify_finished_) {
     delay_until_image_notify_finished_->DocumentChanged(
         element_->GetDocument());
   }
   ClearFailedLoadURL();
   SetImage(0);
 }
 
 }  // namespace blink