Improve handling of environment (viewport) changes

Pass along additional information from a <source> that changes state, so
that it is possible to determine if the change was due to a media
condition changing state or the element being added/removed/mutated. In
the former case have this change the update behavior passed to the image
loader.

In ImageLoader, move the handling of the `suppress_error_events_` flag
into ImageLoader::DispatchErrorEvent() to make it more unified.

Bug: 1233739
Change-Id: Ib2b363e3a28e779981cef9fc2e55fbb86fcf8413
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4365771
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/main@{#1122491}

third_party/blink/renderer/core/html/html_picture_element.cc

@@ -18,10 +18,14 @@ namespace blink {
 HTMLPictureElement::HTMLPictureElement(Document& document)
     : HTMLElement(html_names::kPictureTag, document) {}
 
-void HTMLPictureElement::SourceOrMediaChanged() {
+void HTMLPictureElement::SourceChanged(ImageSourceChangeType change_type) {
+  ImageLoader::UpdateFromElementBehavior update_behavior =
+      change_type == ImageSourceChangeType::kMedia
+          ? ImageLoader::kUpdateSizeChanged
+          : ImageLoader::kUpdateNormal;
   for (HTMLImageElement& image_element :
        Traversal<HTMLImageElement>::ChildrenOf(*this)) {
-    image_element.SelectSourceURL(ImageLoader::kUpdateNormal);
+    image_element.SelectSourceURL(update_behavior);
   }
 }
 

third_party/blink/renderer/core/html/html_picture_element.h

@@ -9,13 +9,25 @@
 
 namespace blink {
 
+// Description of a change to a <source> element.
+enum class ImageSourceChangeType {
+  // A <source> element was added.
+  kAdded,
+  // A <source> element was removed.
+  kRemoved,
+  // An attribute of a <source> element changed.
+  kAttribute,
+  // The 'media' condition of a <source> element changed.
+  kMedia,
+};
+
 class HTMLPictureElement final : public HTMLElement {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
   explicit HTMLPictureElement(Document&);
 
-  void SourceOrMediaChanged();
+  void SourceChanged(ImageSourceChangeType);
   void SourceDimensionChanged();
   void RemoveListenerFromSourceChildren();
   void AddListenerToSourceChildren();

third_party/blink/renderer/core/html/html_source_element.cc

@@ -95,9 +95,9 @@ Node::InsertionNotificationRequest HTMLSourceElement::InsertedInto(
   auto* html_picture_element = parent == insertion_point
                                    ? DynamicTo<HTMLPictureElement>(parent)
                                    : nullptr;
-  if (html_picture_element)
-    html_picture_element->SourceOrMediaChanged();
-
+  if (html_picture_element) {
+    html_picture_element->SourceChanged(ImageSourceChangeType::kAdded);
+  }
   return kInsertionDone;
 }
 
@@ -111,7 +111,7 @@ void HTMLSourceElement::RemovedFrom(ContainerNode& removal_root) {
   if (auto* picture = DynamicTo<HTMLPictureElement>(parent)) {
     RemoveMediaQueryListListener();
     if (was_removed_from_parent)
-      picture->SourceOrMediaChanged();
+      picture->SourceChanged(ImageSourceChangeType::kRemoved);
   }
   HTMLElement::RemovedFrom(removal_root);
 }
@@ -184,14 +184,16 @@ void HTMLSourceElement::ParseAttribute(
     CreateMediaQueryList(params.new_value);
   if (name == html_names::kSrcsetAttr || name == html_names::kSizesAttr ||
       name == html_names::kMediaAttr || name == html_names::kTypeAttr) {
-    if (auto* picture = DynamicTo<HTMLPictureElement>(parentElement()))
-      picture->SourceOrMediaChanged();
+    if (auto* picture = DynamicTo<HTMLPictureElement>(parentElement())) {
+      picture->SourceChanged(ImageSourceChangeType::kAttribute);
+    }
   }
 }
 
 void HTMLSourceElement::NotifyMediaQueryChanged() {
-  if (auto* picture = DynamicTo<HTMLPictureElement>(parentElement()))
-    picture->SourceOrMediaChanged();
+  if (auto* picture = DynamicTo<HTMLPictureElement>(parentElement())) {
+    picture->SourceChanged(ImageSourceChangeType::kMedia);
+  }
 }
 
 void HTMLSourceElement::Trace(Visitor* visitor) const {

third_party/blink/renderer/core/loader/image_loader.cc

@@ -366,6 +366,12 @@ static void ConfigureRequest(
 }
 
 inline void ImageLoader::DispatchErrorEvent() {
+  // The error event should not fire if the image data update is a result of
+  // environment change.
+  // https://html.spec.whatwg.org/C/#the-img-element:the-img-element-55
+  if (suppress_error_events_) {
+    return;
+  }
   // There can be cases where DispatchErrorEvent() is called when there is
   // already a scheduled error event for the previous load attempt.
   // In such cases we cancel the previous event (by overwriting
@@ -804,11 +810,7 @@ void ImageLoader::ImageNotifyFinished(ImageResourceContent* content) {
     if (error && error->IsAccessCheck())
       CrossSiteOrCSPViolationOccurred(AtomicString(error->FailingURL()));
 
-    // The error event should not fire if the image data update is a result of
-    // environment change.
-    // https://html.spec.whatwg.org/C/#the-img-element:the-img-element-55
-    if (!suppress_error_events_)
-      DispatchErrorEvent();
+    DispatchErrorEvent();
     return;
   }