[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..ab295bcdcac2c84588a59b89bf70346545280b69 100644
--- a/third_party/WebKit/Source/core/html/ImageDocument.cpp
+++ b/third_party/WebKit/Source/core/html/ImageDocument.cpp
@@ -172,6 +172,7 @@ void ImageDocumentParser::finish() {
     }
 
     document()->imageUpdated();
+    document()->imageLoaded();
   }
 
   if (!isDetached())
@@ -187,6 +188,9 @@ ImageDocument::ImageDocument(const DocumentInit& initializer)
       m_imageSizeIsKnown(false),
       m_didShrinkImage(false),
       m_shouldShrinkImage(shouldShrinkToFit()),
+      m_imageIsLoaded(false),
+      m_checkerSizeInitialized(false),
+      m_divWidth(0),
       m_shrinkToFitMode(frame()->settings()->viewportEnabled() ? Viewport
                                                                : Desktop) {
   setCompatibilityMode(QuirksMode);
@@ -218,9 +222,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 +247,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 +260,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) {

[pdr., 2016/10/28 21:20:48]

Because many desktops have touchscreens these days, should we unconditionally
add the touch event listeners?

[gone, 2016/10/28 21:36:20]

The touch listeners applied only to viewport mode because of the difference in
zooming methods: desktop doesn't dynamically size the checkers because it can
render them all at the correct size whenever the image is fully zoomed in or
fully zoomed out.

[pdr., 2016/10/28 22:16:50]

ah, sgtm.

+      m_imageElement->addEventListener(EventTypeNames::touchend, listener,
+                                       false);
+      m_imageElement->addEventListener(EventTypeNames::touchcancel, listener,
+                                       false);
+    }
   }
 
   rootElement->appendChild(head);
@@ -346,6 +353,72 @@ void ImageDocument::imageClicked(int x, int y) {
   }
 }
 
+void ImageDocument::imageLoaded() {
+  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) {
+      // The base square size is set to 10 because it rounds nicely for both the
+      // minimum scale (0.1) and maximum scale (5.0).
+      double checkerSize = 10;
+
+      if (m_shrinkToFitMode == Viewport) {
+        // To ensure the checker pattern is visible for large images in
+        // viewports, the checker size must be dynamically adjusted to account
+        // for how much the page is currently being scaled.
+        //
+        // Unfortunately, the page scale is not properly initialized when this

[pdr., 2016/10/28 21:20:48]

This workaround is unfortunate. Can you think of any way to avoid it? For
example, could we assume the initial scale is 1, or always use the width /
divwidth codepath?

[gone, 2016/10/28 21:36:20]

Yeah, I'm not happy with it either, which is why I was trying to push for not
dynamically changing the checker size :/

This is the only way I could think of to figure out what the initial scale
really needed to be... the scale is 1.0 means that the checkers will be exactly
10px * 10px, regardless of how wide the image actually is.  The gigantic 4k X 4k
test image I had, for example, ends up with an initial scale of 0.1.

Oddly, the initial scale for all images on my phone was set to ~0.367 or
something strange when it really needed to be width/divwidth, and it's still not
set properly when the image finishes loading (hence the boolean).  Always
setting the checker size to width/divwidth fails to account for the scale
changing, meaning that the checkers will be enlarged or shrunk with the exact
same scale as the image, and will never try to be a scale appropriate size :/

[pdr., 2016/10/28 22:16:50]

Any idea where 0.367 comes from? Could be worth figuring that out.

I'm not really sure where to start on this.

An idea: updateImageStyle is called from many places. Is the scale wrong on the
first call from all of them? For some background, documents have a "lifecycle"
state machine where we go through several states to render a page
(https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Docum...).
It could be that the scale isn't available until one of those later lifecycle
states, and maybe the call from createDocumentStructure is just too early? You
could try testing if the scale is available when other callsites to
updateImageStyle.

[gone, 2016/10/28 23:42:22]

Did some more experimentation about found that the page scale factor seems to be
set correctly after the call to document()->finishedParsing() — which also
unfortunately detaches the parser from the document() and prevents us from
calling document()->imageLoaded() :/

+        // function is called the first time.  This workaround determines the
+        // initial scale by comparing the size of the image's container against
+        // the viewport's size (see ImageDocument::windowSizeChanged).
+        double scale =
+            m_checkerSizeInitialized
+                ? frame()->host()->visualViewport().scale()
+                : double(frame()->host()->visualViewport().size().width()) /
+                      m_divWidth;
+        checkerSize = std::max(1.0, checkerSize / scale);
+        m_checkerSizeInitialized = true;
+      }
+
+      int roundedSize = static_cast<int>(round(checkerSize));
+      int tileSize = roundedSize * 2;
+
+      imageStyle.append("background-position: 0px 0px, ");
+      imageStyle.append(AtomicString::number(roundedSize));
+      imageStyle.append("px ");
+      imageStyle.append(AtomicString::number(roundedSize));
+      imageStyle.append("px;");
+
+      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%);");
+    }
+  }
+
+  m_imageElement->setAttribute(styleAttr, imageStyle.toAtomicString());
+}
+
 void ImageDocument::imageUpdated() {
   DCHECK(m_imageElement);
 
@@ -422,17 +495,19 @@ void ImageDocument::windowSizeChanged() {
     // 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);
-    m_divElement->setInlineStyleProperty(CSSPropertyWidth, divWidth,
+    m_divWidth = std::max(viewportWidth, maxWidth);
+    m_divElement->setInlineStyleProperty(CSSPropertyWidth, m_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 divHeight = std::max(imageSize.height().toInt(),
-                             (int)(divWidth / viewportAspectRatio));
+    int divHeight =
+        std::max(imageSize.height().toInt(),
+                 static_cast<int>(m_divWidth / viewportAspectRatio));
     m_divElement->setInlineStyleProperty(CSSPropertyHeight, divHeight,
                                          CSSPrimitiveValue::UnitType::Pixels);
+    updateImageStyle();
     return;
   }
 
@@ -495,6 +570,8 @@ 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->isTouchEvent()) {
+    m_doc->updateImageStyle();
   }
 }