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 196 matching lines @@
 }
 
 void ImageLoader::SetImageForImageDocument(ImageResource* new_image_resource) {
   DCHECK(loading_image_document_);
   DCHECK(new_image_resource);
   DCHECK(new_image_resource->GetContent());
 
   image_resource_for_image_document_ = new_image_resource;
   SetImageWithoutConsideringPendingLoadEvent(new_image_resource->GetContent());
 
   // |has_pending_load_event_| is always false and |image_complete_| is

[yhirano, 2017/05/12 10:49:50]

Is this comment stale?

[hiroshige, 2017/05/19 20:56:34]

Done.

   // always true for ImageDocument loading, while the loading is just started.
   // TODO(hiroshige): clean up the behavior of flags. https://crbug.com/719759
-  has_pending_load_event_ = false;
   image_complete_ = true;
 
   // 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::SetImageWithoutConsideringPendingLoadEvent(
     ImageResourceContent* new_image) {
@@ skipping 62 matching lines @@
       Task::Create(this, update_behavior, referrer_policy);
   pending_task_ = task->CreateWeakPtr();
   Microtask::EnqueueMicrotask(
       WTF::Bind(&Task::Run, WTF::Passed(std::move(task))));
   delay_until_do_update_from_element_ =
       IncrementLoadEventDelayCount::Create(element_->GetDocument());
 }
 
 void ImageLoader::UpdateImageState(ImageResourceContent* new_image) {
   image_ = new_image;
-  has_pending_load_event_ = new_image;
   image_complete_ = !new_image;
   delay_until_image_notify_finished_ = nullptr;
 }
 
 void ImageLoader::DoUpdateFromElement(BypassMainWorldBehavior bypass_behavior,
                                       UpdateFromElementBehavior update_behavior,
                                       const KURL& url,
                                       ReferrerPolicy referrer_policy) {
   // FIXME: According to
   // http://www.whatwg.org/specs/web-apps/current-work/multipage/embedded-content.html#the-img-element:the-img-element-55
@@ skipping 238 matching lines @@
   // Update ImageAnimationPolicy for 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_);
 
   if (resource->ErrorOccurred()) {
     LoadEventSender().CancelEvent(this);
     has_pending_load_event_ = 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 18 matching lines @@
     return;
 
   // Only update the layoutObject if it doesn't have an image or if what we have
   // is a complete image.  This prevents flickering in the case where a dynamic
   // change is happening between two images.
   ImageResourceContent* cached_image = image_resource->CachedImage();
   if (image_ != cached_image && (image_complete_ || !cached_image))
     image_resource->SetImageResource(image_.Get());
 }
 
+bool ImageLoader::HasPendingEvent() const {
+  return (image_ && !image_complete_ && !loading_image_document_) ||
+         has_pending_load_event_ || has_pending_error_event_;

[kinuko, 2017/05/12 08:28:24]

This condition is getting slightly complex, maybe worth adding some comment?

'True if it has pending tasks for load/error event, or regular image load
(!loading_image_document_) is in progress (image_ && !image_complete_)'

[hiroshige, 2017/05/19 20:56:35]

Done.

+}
+
 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_ = HasPendingEvent();
   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 67 matching lines @@
   }
   if (delay_until_image_notify_finished_) {
     delay_until_image_notify_finished_->DocumentChanged(
         element_->GetDocument());
   }
   ClearFailedLoadURL();
   ClearImage();
 }
 
 }  // namespace blink