CSSStyleSheetResource should cache decoded text instead of raw bytes

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2003, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2009 Rob Buis (rwlbuis@gmail.com)
  * Copyright (C) 2011 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
@@ skipping 408 matching lines @@
     m_loading = false;
     removePendingSheet();
     if (m_sheet)
       clearSheet();
     return;
   }
 
   // See the comment in PendingScript.cpp about why this check is necessary
   // here, instead of in the resource fetcher. https://crbug.com/500701.
   if (!cachedStyleSheet->errorOccurred() &&
-      m_owner->fastHasAttribute(HTMLNames::integrityAttr) &&
-      cachedStyleSheet->resourceBuffer() &&
-      !SubresourceIntegrity::CheckSubresourceIntegrity(
-          *m_owner, cachedStyleSheet->resourceBuffer()->data(),
-          cachedStyleSheet->resourceBuffer()->size(), KURL(baseURL, href),
-          *cachedStyleSheet)) {
-    m_loading = false;
-    removePendingSheet();
-    notifyLoadedSheetAndAllCriticalSubresources(
-        Node::ErrorOccurredLoadingSubresource);
-    return;
+      !m_owner->fastGetAttribute(HTMLNames::integrityAttr).isEmpty() &&
+      !cachedStyleSheet->integrityMetadata().isEmpty()) {
+    ResourceIntegrityDisposition disposition =
+        cachedStyleSheet->integrityDisposition();
+
+    if (disposition == ResourceIntegrityDisposition::NotChecked)

[Charlie Harrison, 2016/10/05 15:24:49]

nit: A conditional branch just for a DCHECK isn't great style. You can just
combine the conditions within the DCHECK:

DCHECK(disposition != ResourceIntegrityDisposition::NotChecked || 
       cachedStyleSheet->sheetText().isNull());

(too bad DCHECK_IMPLIES got removed from the codebase)

[kouhei (in TOK), 2016/10/06 12:02:36]

Done.

+      DCHECK(!cachedStyleSheet->sheetText().isNull());
+
+    if (disposition == ResourceIntegrityDisposition::NotChecked &&
+        cachedStyleSheet->resourceBuffer()) {
+      if (SubresourceIntegrity::CheckSubresourceIntegrity(
+              *m_owner, cachedStyleSheet->resourceBuffer()->data(),
+              cachedStyleSheet->resourceBuffer()->size(), KURL(baseURL, href),
+              *cachedStyleSheet))
+        disposition = ResourceIntegrityDisposition::Passed;
+      else
+        disposition = ResourceIntegrityDisposition::Failed;
+
+      const_cast<CSSStyleSheetResource*>(cachedStyleSheet)

[Charlie Harrison, 2016/10/05 15:24:48]

The const_cast is not ideal, but I can't think of a way around it without
significant refactors.

Ideas:
 - Change setCSSStyleSheet to return the disposition, and cache it in the
resource from the caller.
 - Call it out for removal when crbug.com/500701 is addressed with a comment.

+          ->setIntegrityDisposition(disposition);
+    }
+
+    if (disposition == ResourceIntegrityDisposition::Failed) {
+      m_loading = false;
+      removePendingSheet();
+      notifyLoadedSheetAndAllCriticalSubresources(
+          Node::ErrorOccurredLoadingSubresource);
+      return;
+    }
   }
 
   CSSParserContext parserContext(m_owner->document(), nullptr, baseURL,
                                  charset);
 
   DEFINE_STATIC_LOCAL(EnumerationHistogram, restoredCachedStyleSheetHistogram,
                       ("Blink.RestoredCachedStyleSheet", 2));
   DEFINE_STATIC_LOCAL(
       EnumerationHistogram, restoredCachedStyleSheet2Histogram,
       ("Blink.RestoredCachedStyleSheet2", StyleSheetCacheStatusCount));
@@ skipping 284 matching lines @@
     removePendingSheet();
 }
 
 DEFINE_TRACE(LinkStyle) {
   visitor->trace(m_sheet);
   LinkResource::trace(visitor);
   ResourceOwner<StyleSheetResource>::trace(visitor);
 }
 
 }  // namespace blink