Do not show image placeholders for CSS sprites

third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc.
  * All rights reserved.
  * Copyright (C) 2013 Google 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 22 matching lines @@
 #include "core/style/CursorData.h"
 #include "core/style/FillLayer.h"
 #include "core/style/FilterOperation.h"
 #include "core/style/StyleFetchedImage.h"
 #include "core/style/StyleFetchedImageSet.h"
 #include "core/style/StyleGeneratedImage.h"
 #include "core/style/StyleImage.h"
 #include "core/style/StyleInvalidImage.h"
 #include "core/style/StylePendingImage.h"
 #include "core/svg/SVGElementProxy.h"
+#include "platform/Length.h"
+#include "platform/loader/fetch/FetchParameters.h"
 #include "platform/loader/fetch/ResourceFetcher.h"
 
 namespace blink {
 
 ElementStyleResources::ElementStyleResources(Document& document,
                                              float device_scale_factor)
     : document_(&document), device_scale_factor_(device_scale_factor) {}
 
 StyleImage* ElementStyleResources::GetStyleImage(CSSPropertyID property,
                                                  const CSSValue& value) {
@@ skipping 54 matching lines @@
       computed_style->MutableFilter().Operations();
   for (auto& filter_operation : filter_operations) {
     if (filter_operation->GetType() != FilterOperation::REFERENCE)
       continue;
     ReferenceFilterOperation& reference_operation =
         ToReferenceFilterOperation(*filter_operation);
     reference_operation.ElementProxy().Resolve(*document_);
   }
 }
 
+static bool ComputedStyleMayBeCSSSpriteBackgroundImage(
+    const ComputedStyle& style) {
+  // Simple heuristic to guess if CSS background image is used to create CSS
+  // sprites. For a legit background image it's very likely the height or the
+  // width will be set to auto. For CSS sprite image, height or width will
+  // probably be specified.
+  return style.HasBackgroundImage() &&
+         (!style.LogicalHeight().IsAuto() || !style.LogicalWidth().IsAuto());
+}
+
+static FetchParameters::PlaceholderImageRequestType
+GetPlaceholderImageRequestType(const ComputedStyle& style) {

[kouhei (in TOK), 2017/04/24 00:02:19]

I think we should simply inline the logic here. I found the name
"GetPlaceholderImageRequestType" too generic and hard to guess what the function
is doing without reading it.

[Raj, 2017/04/28 03:43:26]

Done.

+  return ComputedStyleMayBeCSSSpriteBackgroundImage(style)
+             ? FetchParameters::kDisallowPlaceholder
+             : FetchParameters::kAllowPlaceholder;
+}
+
 StyleImage* ElementStyleResources::LoadPendingImage(
     ComputedStyle* style,
     StylePendingImage* pending_image,
+    FetchParameters::PlaceholderImageRequestType placeholder_image_request_type,
     CrossOriginAttributeValue cross_origin) {
-  if (CSSImageValue* image_value = pending_image->CssImageValue())
-    return image_value->CacheImage(*document_, cross_origin);
+  if (CSSImageValue* image_value = pending_image->CssImageValue()) {
+    return image_value->CacheImage(*document_, cross_origin,
+                                   placeholder_image_request_type);
+  }
 
   if (CSSPaintValue* paint_value = pending_image->CssPaintValue()) {
     StyleGeneratedImage* image = StyleGeneratedImage::Create(*paint_value);
     style->AddPaintImage(image);
     return image;
   }
 
   if (CSSImageGeneratorValue* image_generator_value =
           pending_image->CssImageGeneratorValue()) {
     image_generator_value->LoadSubimages(*document_);
     return StyleGeneratedImage::Create(*image_generator_value);
   }
 
-  if (CSSImageSetValue* image_set_value = pending_image->CssImageSetValue())
+  if (CSSImageSetValue* image_set_value = pending_image->CssImageSetValue()) {
     return image_set_value->CacheImage(*document_, device_scale_factor_,
-                                       cross_origin);
+                                       cross_origin,
+                                       placeholder_image_request_type);
+  }
 
   NOTREACHED();
   return nullptr;
 }
 
 void ElementStyleResources::LoadPendingImages(ComputedStyle* style) {
+  FetchParameters::PlaceholderImageRequestType placeholder_image_request_type =
+      GetPlaceholderImageRequestType(*style);

[kouhei (in TOK), 2017/04/24 00:02:19]

IIUC, this decides place_holder_image_request_type_ to be applied on all
image-holding CSS value on the "style", which will disable place_holder on
non-bg image if the style has css bg sprite.

I think what we want to achieve here is disable place_holder on bg image only.

[Raj, 2017/04/28 03:43:27]

Done.

+
   // We must loop over the properties and then look at the style to see if
   // a pending image exists, and only load that image. For example:
   //
   // <style>
   //    div { background-image: url(a.png); }
   //    div { background-image: url(b.png); }
   //    div { background-image: none; }
   // </style>
   // <div></div>
   //
   // We call styleImage() for both a.png and b.png adding the
   // CSSPropertyBackgroundImage property to the pending_image_properties_ set,
   // then we null out the background image because of the "none".
   //
   // If we eagerly loaded the images we'd fetch a.png, even though it's not
   // used. If we didn't null check below we'd crash since the none actually
   // removed all background images.
 
   for (CSSPropertyID property : pending_image_properties_) {
     switch (property) {
       case CSSPropertyBackgroundImage: {
         for (FillLayer* background_layer = &style->AccessBackgroundLayers();
              background_layer; background_layer = background_layer->Next()) {
           if (background_layer->GetImage() &&
-              background_layer->GetImage()->IsPendingImage())
+              background_layer->GetImage()->IsPendingImage()) {
             background_layer->SetImage(LoadPendingImage(
-                style, ToStylePendingImage(background_layer->GetImage())));
+                style, ToStylePendingImage(background_layer->GetImage()),
+                placeholder_image_request_type));

[kouhei (in TOK), 2017/04/24 00:02:19]

Can we fill in place_holder_image_request_type here only

[Raj, 2017/04/28 03:43:26]

Done.

+          }
         }
         break;
       }
       case CSSPropertyContent: {
         for (ContentData* content_data =
                  const_cast<ContentData*>(style->GetContentData());
              content_data; content_data = content_data->Next()) {
           if (content_data->IsImage()) {
             StyleImage* image = ToImageContentData(content_data)->GetImage();
-            if (image->IsPendingImage())
+            if (image->IsPendingImage()) {
               ToImageContentData(content_data)
-                  ->SetImage(
-                      LoadPendingImage(style, ToStylePendingImage(image)));
+                  ->SetImage(LoadPendingImage(style, ToStylePendingImage(image),
+                                              placeholder_image_request_type));

[kouhei (in TOK), 2017/04/24 00:02:19]

and use FetchParameters::kAllowPlaceholder here and below?

[Raj, 2017/04/28 03:43:27]

Done.

+            }
           }
         }
         break;
       }
       case CSSPropertyCursor: {
         if (CursorList* cursor_list = style->Cursors()) {
           for (size_t i = 0; i < cursor_list->size(); ++i) {
             CursorData& current_cursor = cursor_list->at(i);
             if (StyleImage* image = current_cursor.GetImage()) {
-              if (image->IsPendingImage())
+              if (image->IsPendingImage()) {
                 current_cursor.SetImage(
-                    LoadPendingImage(style, ToStylePendingImage(image)));
+                    LoadPendingImage(style, ToStylePendingImage(image),
+                                     placeholder_image_request_type));
+              }
             }
           }
         }
         break;
       }
       case CSSPropertyListStyleImage: {
         if (style->ListStyleImage() &&
-            style->ListStyleImage()->IsPendingImage())
+            style->ListStyleImage()->IsPendingImage()) {
           style->SetListStyleImage(LoadPendingImage(
-              style, ToStylePendingImage(style->ListStyleImage())));
+              style, ToStylePendingImage(style->ListStyleImage()),
+              placeholder_image_request_type));
+        }
         break;
       }
       case CSSPropertyBorderImageSource: {
         if (style->BorderImageSource() &&
-            style->BorderImageSource()->IsPendingImage())
+            style->BorderImageSource()->IsPendingImage()) {
           style->SetBorderImageSource(LoadPendingImage(
-              style, ToStylePendingImage(style->BorderImageSource())));
+              style, ToStylePendingImage(style->BorderImageSource()),
+              placeholder_image_request_type));
+        }
         break;
       }
       case CSSPropertyWebkitBoxReflect: {
         if (StyleReflection* reflection = style->BoxReflect()) {
           const NinePieceImage& mask_image = reflection->Mask();
           if (mask_image.GetImage() &&
               mask_image.GetImage()->IsPendingImage()) {
             StyleImage* loaded_image = LoadPendingImage(
-                style, ToStylePendingImage(mask_image.GetImage()));
+                style, ToStylePendingImage(mask_image.GetImage()),
+                FetchParameters::kAllowPlaceholder);
             reflection->SetMask(NinePieceImage(
                 loaded_image, mask_image.ImageSlices(), mask_image.Fill(),
                 mask_image.BorderSlices(), mask_image.Outset(),
                 mask_image.HorizontalRule(), mask_image.VerticalRule()));
           }
         }
         break;
       }
       case CSSPropertyWebkitMaskBoxImageSource: {
         if (style->MaskBoxImageSource() &&
-            style->MaskBoxImageSource()->IsPendingImage())
+            style->MaskBoxImageSource()->IsPendingImage()) {
           style->SetMaskBoxImageSource(LoadPendingImage(
-              style, ToStylePendingImage(style->MaskBoxImageSource())));
+              style, ToStylePendingImage(style->MaskBoxImageSource()),
+              placeholder_image_request_type));
+        }
         break;
       }
       case CSSPropertyWebkitMaskImage: {
         for (FillLayer* mask_layer = &style->AccessMaskLayers(); mask_layer;
              mask_layer = mask_layer->Next()) {
           if (mask_layer->GetImage() &&
-              mask_layer->GetImage()->IsPendingImage())
+              mask_layer->GetImage()->IsPendingImage()) {
             mask_layer->SetImage(LoadPendingImage(
-                style, ToStylePendingImage(mask_layer->GetImage())));
+                style, ToStylePendingImage(mask_layer->GetImage()),
+                placeholder_image_request_type));
+          }
         }
         break;
       }
       case CSSPropertyShapeOutside:
         if (style->ShapeOutside() && style->ShapeOutside()->GetImage() &&
-            style->ShapeOutside()->GetImage()->IsPendingImage())
+            style->ShapeOutside()->GetImage()->IsPendingImage()) {
           style->ShapeOutside()->SetImage(LoadPendingImage(
               style, ToStylePendingImage(style->ShapeOutside()->GetImage()),
-              kCrossOriginAttributeAnonymous));
+              placeholder_image_request_type, kCrossOriginAttributeAnonymous));
+        }
         break;
       default:
         NOTREACHED();
     }
   }
 }
 
 void ElementStyleResources::LoadPendingResources(
     ComputedStyle* computed_style) {
   LoadPendingImages(computed_style);
   LoadPendingSVGDocuments(computed_style);
 }
 
 }  // namespace blink