MHTML: Allows 'data:' URLs to be processed using normal request processing.

third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp

 /*
     Copyright (C) 1998 Lars Knoll (knoll@mpi-hd.mpg.de)
     Copyright (C) 2001 Dirk Mueller (mueller@kde.org)
     Copyright (C) 2002 Waldo Bastian (bastian@kde.org)
     Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
     Copyright (C) 2009 Torch Mobile Inc. http://www.torchmobile.com/
 
     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
@@ skipping 424 matching lines @@
             activityLogger = V8DOMActivityLogger::currentActivityLoggerIfIsolatedWorld();
 
         if (activityLogger) {
             Vector<String> argv;
             argv.append(Resource::resourceTypeToString(factory.type(), request.options().initiatorInfo));
             argv.append(request.url());
             activityLogger->logEvent("blinkRequestResource", argv.size(), argv.data());
         }
     }
 
-    bool isStaticData = request.resourceRequest().url().protocolIsData() || substituteData.isValid() || m_archive;
+    bool isDataUrl = request.resourceRequest().url().protocolIsData();
+    bool isStaticData = isDataUrl || substituteData.isValid() || m_archive;
     Resource* resource(nullptr);
     if (isStaticData) {
         resource = resourceForStaticData(request, factory, substituteData);
-        // Abort the request if the archive doesn't contain the resource.
-        if (!resource && m_archive)
+        // Abort the request if the archive doesn't contain the resource, except

[Dmitry Titov, 2016/09/19 19:54:16]

I still feel the resourceForStaticData should succeed in this case. I think not
loading fonts before they are used is stemming from the desire to avoid actual
network load of large fornts that are mentioned by URL in the stylesheet. There
are stylesheets out there that are listing a bunch of font resources by URL but
the page ends up using one or none etc. In those scenarios, we should avoid
loading. 

As for fonts present in archive as data: URLs, they are already here and it
doesn't make much sense to differentiate them from say images in the same
archive.

Lets discuss a bit offline, perhaps I'm missing something.

[dewittj, 2016/09/20 22:44:55]

The CSS spec says that fonts should not be "downloaded" unless referenced:
https://www.w3.org/TR/css-fonts-3/#font-face-loading

Chrome seems to interpret that as not processing the font - no exceptions etc -
until the font is referenced.  There is a test that checks this behavior for
fonts specified as data: URLs.

It would be better if resourceForStaticData could return a resource that'd
finish loading asynchronously.  Today it calls finish() directly, which is what
triggers the console exception.

I believe that if we just fall through to createResourceForLoading, we will use
the net stack to decode and finish the font resource loading, which implies the
desired async behavior.

+japhet - do you have an opinion on how we should enforce font async loading,
while being certain that archives never can access the actual network?

[Nate Chapin, 2016/09/21 19:30:02]

What if resourceForStaticData() checks resourceNeedsLoad(), and if it returns
false, creates the Resource* but doesn't initialize it? I don't know if that
will catch any cases besides data: urls, but assuming it was data: urls only,
then you could just have startLoad() check whether this is a data: url and do
the parse. That requires refactoring resourceForStaticData() a bunch, though, as
you'd need to pull populating the Resource into a helper.

Having written this idea out, I'm not skeptical that it's cleaner than what you
have here.

+        // in the case of data URLs which might have resources such as fonts
+        // that need to be decoded only on demand.  These data URLs are allowed
+        // to be processed using the normal ResourceFetcher machinery.
+        if (!resource && !isDataUrl && m_archive)
             return nullptr;
     }
     if (!resource)
         resource = memoryCache()->resourceForURL(request.url(), getCacheIdentifier());
 
     // See if we can use an existing resource from the cache. If so, we need to move it to be load blocking.
     moveCachedNonBlockingResourceToBlocking(resource, request);
 
     const RevalidationPolicy policy = determineRevalidationPolicy(factory.type(), request, resource, isStaticData);
     TRACE_EVENT_INSTANT1("blink", "ResourceFetcher::determineRevalidationPolicy", TRACE_EVENT_SCOPE_THREAD, "revalidationPolicy", policy);
@@ skipping 872 matching lines @@
     visitor->trace(m_context);
     visitor->trace(m_archive);
     visitor->trace(m_loaders);
     visitor->trace(m_nonBlockingLoaders);
     visitor->trace(m_documentResources);
     visitor->trace(m_preloads);
     visitor->trace(m_resourceTimingInfoMap);
 }
 
 } // namespace blink