Collapse images disallowed by the Safe Browsing Subresource Filter.

third_party/WebKit/Source/core/html/HTMLImageElement.cpp

Index: third_party/WebKit/Source/core/html/HTMLImageElement.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLImageElement.cpp b/third_party/WebKit/Source/core/html/HTMLImageElement.cpp
index 01fc2ac62b9849ede2e4d98b0d73447fdedc8672..8da11f5d997f2583f2c9bb9b117300fd99fb8617 100644
--- a/third_party/WebKit/Source/core/html/HTMLImageElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLImageElement.cpp
@@ -93,9 +93,9 @@ HTMLImageElement::HTMLImageElement(Document& document,
       m_imageLoader(HTMLImageLoader::create(this)),
       m_imageDevicePixelRatio(1.0f),
       m_source(nullptr),
+      m_layoutDisposition(LayoutDisposition::PrimaryContent),
       m_formWasSetByParser(false),
       m_elementCreatedByParser(createdByParser),
-      m_useFallbackContent(false),
       m_isFallbackImage(false),
       m_referrerPolicy(ReferrerPolicyDefault) {
   setHasCustomStyleCallbacks();
@@ -339,6 +339,11 @@ ImageCandidate HTMLImageElement::findBestFitImageFromPictureParent() {
   return ImageCandidate();
 }
 
+bool HTMLImageElement::layoutObjectIsNeeded(const ComputedStyle& style) {
+  return m_layoutDisposition != LayoutDisposition::Collapsed &&
+         HTMLElement::layoutObjectIsNeeded(style);
+}
+
 LayoutObject* HTMLImageElement::createLayoutObject(const ComputedStyle& style) {
   const ContentData* contentData = style.contentData();
   if (contentData && contentData->isImage()) {
@@ -349,13 +354,19 @@ LayoutObject* HTMLImageElement::createLayoutObject(const ComputedStyle& style) {
       return LayoutObject::createObject(this, style);
   }
 
-  if (m_useFallbackContent)
-    return new LayoutBlockFlow(this);
-
-  LayoutImage* image = new LayoutImage(this);
-  image->setImageResource(LayoutImageResource::create());
-  image->setImageDevicePixelRatio(m_imageDevicePixelRatio);
-  return image;
+  switch (m_layoutDisposition) {

[dominicc (has gone to gerrit), 2016/12/07 01:51:42]

Sadness, it seems the compiler doesn't work out that these cases are exhaustive.
I guess we need to add default and have LayoutDisposition::Collapsed fall
through to that.

[engedy, 2016/12/07 09:37:41]

Done.

+    case LayoutDisposition::FallbackContent:
+      return new LayoutBlockFlow(this);
+    case LayoutDisposition::PrimaryContent: {
+      LayoutImage* image = new LayoutImage(this);
+      image->setImageResource(LayoutImageResource::create());
+      image->setImageDevicePixelRatio(m_imageDevicePixelRatio);
+      return image;
+    }
+    case LayoutDisposition::Collapsed:
+      NOTREACHED();
+      return nullptr;
+  }
 }
 
 void HTMLImageElement::attachLayoutTree(const AttachContext& context) {
@@ -770,6 +781,7 @@ void HTMLImageElement::selectSourceURL(
         fastGetAttribute(srcAttr), fastGetAttribute(srcsetAttr), &document());
     setBestFitURLAndDPRFromImageCandidate(candidate);
   }
+
   imageLoader().updateFromElement(behavior, m_referrerPolicy);
 
   // Images such as data: uri's can return immediately and may already have
@@ -801,7 +813,7 @@ void HTMLImageElement::selectSourceURL(
   if ((imageHasLoaded && imageHasImage) || imageStillLoading || imageIsDocument)
     ensurePrimaryContent();
   else
-    ensureFallbackContent();
+    ensureCollapsedOrFallbackContent();
 }
 
 const KURL& HTMLImageElement::sourceURL() const {
@@ -813,49 +825,59 @@ void HTMLImageElement::didAddUserAgentShadowRoot(ShadowRoot&) {
 }
 
 void HTMLImageElement::ensureFallbackForGeneratedContent() {
-  setUseFallbackContent();
-  reattachFallbackContent();
+  // FIXME: The special casing for generated content in createLayoutObject

[dominicc (has gone to gerrit), 2016/12/07 01:51:42]

TODO(whom): [what to fix] [when]

If this is just "this is sad but we're going to live with it" then I'd just
write a comment to that effect, no FIXME/TODO.

[engedy, 2016/12/07 09:37:41]

At this point I am optimistic that this should be fixable. Filed and referenced
a bug.

+  // breaks the invariant that the layout object attached to this element will
+  // always be appropriate for |m_layoutDisposition|. Force recreate it.
+  // See example: images/content-url-broken-image-with-alt-text.html.
+  setLayoutDisposition(LayoutDisposition::FallbackContent,
+                       true /* forceReattach */);
 }
 
-void HTMLImageElement::ensureFallbackContent() {
-  if (m_useFallbackContent || m_isFallbackImage)
+void HTMLImageElement::ensureCollapsedOrFallbackContent() {
+  if (m_isFallbackImage)
     return;
-  setUseFallbackContent();
-  reattachFallbackContent();
+
+  if (imageLoader().image() &&

[dominicc (has gone to gerrit), 2016/12/07 01:51:42]

Seems OK, but maybe

bool explainingVar = that expression;
setLayoutDisposition(explainingVar ? ... : ...);

is both more succinct and more informative because of the name of explainingVar?

[engedy, 2016/12/07 09:37:40]

Done.

+      imageLoader().image()->resourceError().shouldCollapseInitiator()) {
+    setLayoutDisposition(LayoutDisposition::Collapsed);
+  } else {
+    setLayoutDisposition(LayoutDisposition::FallbackContent);
+  }
 }
 
 void HTMLImageElement::ensurePrimaryContent() {
-  if (!m_useFallbackContent)
-    return;
-  m_useFallbackContent = false;
-  reattachFallbackContent();
+  setLayoutDisposition(LayoutDisposition::PrimaryContent);
 }
 
-void HTMLImageElement::reattachFallbackContent() {
+void HTMLImageElement::setLayoutDisposition(LayoutDisposition layoutDisposition,
+                                            bool forceReattach) {
+  if (m_layoutDisposition == layoutDisposition && !forceReattach)
+    return;
+
+  m_layoutDisposition = layoutDisposition;
+
   // This can happen inside of attachLayoutTree() in the middle of a recalcStyle
   // so we need to reattach synchronously here.
-  if (document().inStyleRecalc())
+  if (document().inStyleRecalc()) {
     reattachLayoutTree();
-  else
+  } else {
+    if (m_layoutDisposition == LayoutDisposition::FallbackContent) {
+      EventDispatchForbiddenScope::AllowUserAgentEvents allowEvents;
+      ensureUserAgentShadowRoot();
+    }
     lazyReattachIfAttached();
+  }
 }
 
 PassRefPtr<ComputedStyle> HTMLImageElement::customStyleForLayoutObject() {
-  RefPtr<ComputedStyle> newStyle = originalStyleForLayoutObject();
-
-  if (!m_useFallbackContent)
-    return newStyle;
-
-  RefPtr<ComputedStyle> style = ComputedStyle::clone(*newStyle);
-  return HTMLImageFallbackHelper::customStyleForAltText(*this, style);
-}
-
-void HTMLImageElement::setUseFallbackContent() {
-  m_useFallbackContent = true;
-  if (document().inStyleRecalc())
-    return;
-  EventDispatchForbiddenScope::AllowUserAgentEvents allowEvents;
-  ensureUserAgentShadowRoot();
+  switch (m_layoutDisposition) {
+    case LayoutDisposition::PrimaryContent:  // Fall through.
+    case LayoutDisposition::Collapsed:
+      return originalStyleForLayoutObject();
+    case LayoutDisposition::FallbackContent:
+      return HTMLImageFallbackHelper::customStyleForAltText(
+          *this, ComputedStyle::clone(*originalStyleForLayoutObject()));
+  }
 }
 
 bool HTMLImageElement::isOpaque() const {