[Blink] Display images on a dark background

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

Index: third_party/WebKit/Source/core/html/ImageDocument.cpp
diff --git a/third_party/WebKit/Source/core/html/ImageDocument.cpp b/third_party/WebKit/Source/core/html/ImageDocument.cpp
index a3d63fca4c7b905591e9d751fec82c526f22ccf4..f9debf7b2f6dbe9d7ace18075c0f853cf2dd538d 100644
--- a/third_party/WebKit/Source/core/html/ImageDocument.cpp
+++ b/third_party/WebKit/Source/core/html/ImageDocument.cpp
@@ -54,6 +54,14 @@
 
 using namespace std;
 
+namespace {
+
+// The base square size is set to 10 because it rounds nicely for both the
+// minimum scale (0.1) and maximum scale (5.0).
+int gBaseCheckerSize = 10;

[pdr., 2016/11/02 22:54:49]

Nit: since this is a constant, wdyt of:
const int kBaseCheckerSize = 10;

[gone, 2016/11/03 01:28:41]

Ah forgot what the proper syntax was here.  Done.

+
+}  // namespace
+
 namespace blink {
 
 using namespace HTMLNames;
@@ -123,6 +131,13 @@ static String imageTitle(const String& filename, const IntSize& size) {
   return result.toString();
 }
 
+static LayoutSize cachedImageSize(HTMLImageElement* element) {
+  DCHECK(element->cachedImage());
+  return element->cachedImage()->imageSize(
+      LayoutObject::shouldRespectImageOrientation(element->layoutObject()),
+      1.0f);
+}
+
 void ImageDocumentParser::appendBytes(const char* data, size_t length) {
   if (!length)
     return;
@@ -155,10 +170,7 @@ void ImageDocumentParser::finish() {
     // Report the natural image size in the page title, regardless of zoom
     // level.  At a zoom level of 1 the image is guaranteed to have an integer
     // size.
-    IntSize size = flooredIntSize(
-        cachedImage->imageSize(LayoutObject::shouldRespectImageOrientation(
-                                   document()->imageElement()->layoutObject()),
-                               1.0f));
+    IntSize size = flooredIntSize(cachedImageSize(document()->imageElement()));
     if (size.width()) {
       // Compute the title, we use the decoded filename of the resource, falling
       // back on the (decoded) hostname if there is no path.
@@ -172,6 +184,7 @@ void ImageDocumentParser::finish() {
     }
 
     document()->imageUpdated();
+    document()->imageLoaded();
   }
 
   if (!isDetached())
@@ -187,6 +200,8 @@ ImageDocument::ImageDocument(const DocumentInit& initializer)
       m_imageSizeIsKnown(false),
       m_didShrinkImage(false),
       m_shouldShrinkImage(shouldShrinkToFit()),
+      m_imageIsLoaded(false),
+      m_checkerSize(gBaseCheckerSize),
       m_shrinkToFitMode(frame()->settings()->viewportEnabled() ? Viewport
                                                                : Desktop) {
   setCompatibilityMode(QuirksMode);
@@ -218,9 +233,9 @@ void ImageDocument::createDocumentStructure() {
 
   if (shouldShrinkToFit()) {
     // Display the image prominently centered in the frame.
-    body->setAttribute(styleAttr, "margin: 0px;");
+    body->setAttribute(styleAttr, "margin: 0px; background: #0e0e0e;");
 
-    // See w3c example on how to centering an element:
+    // See w3c example on how to center an element:
     // https://www.w3.org/Style/Examples/007/center.en.html
     m_divElement = HTMLDivElement::create(*this);
     m_divElement->setAttribute(styleAttr,
@@ -243,12 +258,8 @@ void ImageDocument::createDocumentStructure() {
 
   willInsertBody();
 
-  StringBuilder imageStyle;
-  imageStyle.append("-webkit-user-select: none;");
-  if (shouldShrinkToFit() && m_shrinkToFitMode == Viewport)
-    imageStyle.append("max-width: 100%");
   m_imageElement = HTMLImageElement::create(*this);
-  m_imageElement->setAttribute(styleAttr, imageStyle.toAtomicString());
+  updateImageStyle();
   m_imageElement->setLoadingImageDocument();
   m_imageElement->setSrc(url().getString());
   body->appendChild(m_imageElement.get());
@@ -260,9 +271,16 @@ void ImageDocument::createDocumentStructure() {
     // Add event listeners
     EventListener* listener = ImageEventListener::create(this);
     if (LocalDOMWindow* domWindow = this->domWindow())
-      domWindow->addEventListener("resize", listener, false);
-    if (m_shrinkToFitMode == Desktop)
-      m_imageElement->addEventListener("click", listener, false);
+      domWindow->addEventListener(EventTypeNames::resize, listener, false);
+
+    if (m_shrinkToFitMode == Desktop) {
+      m_imageElement->addEventListener(EventTypeNames::click, listener, false);
+    } else if (m_shrinkToFitMode == Viewport) {
+      m_imageElement->addEventListener(EventTypeNames::touchend, listener,
+                                       false);
+      m_imageElement->addEventListener(EventTypeNames::touchcancel, listener,
+                                       false);
+    }
   }
 
   rootElement->appendChild(head);
@@ -301,11 +319,7 @@ void ImageDocument::resizeImageToFit() {
   if (!m_imageElement || m_imageElement->document() != this)
     return;
 
-  DCHECK(m_imageElement->cachedImage());
-  LayoutSize imageSize = m_imageElement->cachedImage()->imageSize(
-      LayoutObject::shouldRespectImageOrientation(
-          m_imageElement->layoutObject()),
-      1.f);
+  LayoutSize imageSize = cachedImageSize(m_imageElement);
 
   const float scale = this->scale();
   m_imageElement->setWidth(static_cast<int>(imageSize.width() * scale));
@@ -346,6 +360,73 @@ void ImageDocument::imageClicked(int x, int y) {
   }
 }
 
+void ImageDocument::imageLoaded() {
+  if (!shouldShrinkToFit())
+    return;
+
+  int checkerSize = gBaseCheckerSize;
+  if (m_shrinkToFitMode == Viewport) {
+    int viewportWidth = frame()->host()->visualViewport().size().width();
+    checkerSize = static_cast<int>(gBaseCheckerSize *
+                                   (double)calculateDivWidth() / viewportWidth);
+  }
+  m_checkerSize = checkerSize;
+
+  m_imageIsLoaded = true;
+  updateImageStyle();
+}
+
+void ImageDocument::updateImageStyle() {
+  StringBuilder imageStyle;
+  imageStyle.append("-webkit-user-select: none;");
+  if (shouldShrinkToFit()) {
+    if (m_shrinkToFitMode == Viewport) {
+      imageStyle.append("max-width: 100%;");
+    }
+
+    // Once the image has fully loaded, it is displayed atop a checkerboard to
+    // show transparency more faithfully.  The pattern is generated via CSS.
+    if (m_imageIsLoaded) {
+      if (m_shrinkToFitMode == Viewport && hasFinishedParsing()) {
+        double newCheckerSize = gBaseCheckerSize;

[pdr., 2016/11/02 22:54:49]

I think your new approach here is reasonable.

+        if (m_shrinkToFitMode == Viewport) {
+          // To ensure the checker pattern is visible for large images in
+          // viewports, the checker size is dynamically adjusted to account for
+          // how much the page is currently being scaled.
+          double scale = frame()->host()->visualViewport().scale();
+          newCheckerSize = std::max(1.0, newCheckerSize / scale);
+        }
+
+        m_checkerSize = static_cast<int>(round(newCheckerSize));

[pdr., 2016/11/02 22:54:49]

Nit: just "m_checkerSize = round(newCheckerSize);" ?

[gone, 2016/11/03 01:28:41]

Done.

+      }
+
+      imageStyle.append("background-position: 0px 0px, ");
+      imageStyle.append(AtomicString::number(m_checkerSize));
+      imageStyle.append("px ");
+      imageStyle.append(AtomicString::number(m_checkerSize));
+      imageStyle.append("px;");
+
+      int tileSize = m_checkerSize * 2;
+      imageStyle.append("background-size: ");
+      imageStyle.append(AtomicString::number(tileSize));
+      imageStyle.append("px ");
+      imageStyle.append(AtomicString::number(tileSize));
+      imageStyle.append("px;");
+
+      imageStyle.append(
+          "background-color: white;"
+          "background-image:"
+          "linear-gradient(45deg, #eee 25%, transparent 25%, transparent 75%, "
+          "#eee 75%, #eee 100%),"
+          "linear-gradient(45deg, #eee 25%, transparent 25%, transparent 75%, "
+          "#eee 75%, #eee 100%);");
+    }
+  }
+
+  if (m_imageElement->getAttribute(styleAttr) != imageStyle.toAtomicString())

[pdr., 2016/11/02 22:54:49]

This large string comparison is kinda unfortunate. WDYT about storing off the
values instead, or just leaving this as a TODO for now?

[gone, 2016/11/03 01:28:41]

After figuring out how to avoid the string comparison, and some slight
rearranging to move the body of imageLoaded() as an else clause to the
hasFinishedParsing() conditional above, I've ended up with something similar to
PS7 lol...  I guess we at least understand what's going on a lot better, and it
avoids over-updating?

+    m_imageElement->setAttribute(styleAttr, imageStyle.toAtomicString());
+}
+
 void ImageDocument::imageUpdated() {
   DCHECK(m_imageElement);
 
@@ -377,10 +458,7 @@ void ImageDocument::restoreImageSize() {
     return;
 
   DCHECK(m_imageElement->cachedImage());
-  LayoutSize imageSize = m_imageElement->cachedImage()->imageSize(
-      LayoutObject::shouldRespectImageOrientation(
-          m_imageElement->layoutObject()),
-      1.0f);
+  LayoutSize imageSize = cachedImageSize(m_imageElement);
   m_imageElement->setWidth(imageSize.width().toInt());
   m_imageElement->setHeight(imageSize.height().toInt());
 
@@ -397,40 +475,43 @@ bool ImageDocument::imageFitsInWindow() const {
   return this->scale() >= 1;
 }
 
+int ImageDocument::calculateDivWidth() {
+  // Zooming in and out of an image being displayed within a viewport is done
+  // by changing the page scale factor of the page instead of changing the
+  // size of the image.  The size of the image is set so that:
+  // * Images wider than the viewport take the full width of the screen.
+  // * Images taller than the viewport are initially aligned with the top of
+  //   of the frame.
+  // * Images smaller in either dimension are centered along that axis.
+  LayoutSize imageSize = cachedImageSize(m_imageElement);
+  int viewportWidth = frame()->host()->visualViewport().size().width();
+
+  // For huge images, minimum-scale=0.1 is still too big on small screens.
+  // Set the <div> width so that the image will shrink to fit the width of the
+  // screen when the scale is minimum.
+  int maxWidth = std::min(imageSize.width().toInt(), viewportWidth * 10);
+  return std::max(viewportWidth, maxWidth);
+}
+
 void ImageDocument::windowSizeChanged() {
   if (!m_imageElement || !m_imageSizeIsKnown ||
       m_imageElement->document() != this)
     return;
 
   if (m_shrinkToFitMode == Viewport) {
-    // Zooming in and out of an image being displayed within a viewport is done
-    // by changing the page scale factor of the page instead of changing the
-    // size of the image.  The size of the image is set so that:
-    // * Images wider than the viewport take the full width of the screen.
-    // * Images taller than the viewport are initially aligned with the top of
-    //   of the frame.
-    // * Images smaller in either dimension are centered along that axis.
-    LayoutSize imageSize = m_imageElement->cachedImage()->imageSize(
-        LayoutObject::shouldRespectImageOrientation(
-            m_imageElement->layoutObject()),
-        1.f);
-    int viewportWidth = frame()->host()->visualViewport().size().width();
-    int viewportHeight = frame()->host()->visualViewport().size().height();
-    float viewportAspectRatio = (float)viewportWidth / viewportHeight;
-
-    // For huge images, minimum-scale=0.1 is still too big on small screens.
-    // Set the <div> width so that the image will shrink to fit the width of the
-    // screen when the scale is minimum.
-    int maxWidth = std::min(imageSize.width().toInt(), viewportWidth * 10);
-    int divWidth = std::max(viewportWidth, maxWidth);
+    LayoutSize imageSize = cachedImageSize(m_imageElement);
+    int divWidth = calculateDivWidth();
     m_divElement->setInlineStyleProperty(CSSPropertyWidth, divWidth,
                                          CSSPrimitiveValue::UnitType::Pixels);
 
     // Explicitly set the height of the <div> containing the <img> so that it
     // can display the full image without shrinking it, allowing a full-width
     // reading mode for normal-width-huge-height images.
+    int viewportWidth = frame()->host()->visualViewport().size().width();
+    int viewportHeight = frame()->host()->visualViewport().size().height();
+    float viewportAspectRatio = (float)viewportWidth / viewportHeight;

[pdr., 2016/11/02 22:54:49]

Super nit: float viewportAspectRatio =
frame()->host()->visualViewport().size().aspectRatio();

[gone, 2016/11/03 01:28:41]

Done.

     int divHeight = std::max(imageSize.height().toInt(),
-                             (int)(divWidth / viewportAspectRatio));
+                             static_cast<int>(divWidth / viewportAspectRatio));
     m_divElement->setInlineStyleProperty(CSSPropertyHeight, divHeight,
                                          CSSPrimitiveValue::UnitType::Pixels);
     return;
@@ -495,6 +576,10 @@ void ImageEventListener::handleEvent(ExecutionContext*, Event* event) {
   } else if (event->type() == EventTypeNames::click && event->isMouseEvent()) {
     MouseEvent* mouseEvent = toMouseEvent(event);
     m_doc->imageClicked(mouseEvent->x(), mouseEvent->y());
+  } else if ((event->type() == EventTypeNames::touchend ||
+              event->type() == EventTypeNames::touchcancel) &&
+             event->isTouchEvent()) {
+    m_doc->updateImageStyle();
   }
 }