[DocWriteBlocking] Add feature to block doc.written scripts if 2g-like network

third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 76 matching lines @@
     String message = "A Parser-blocking, cross-origin script, " + url + ", is invoked via document.write. This may be blocked by the browser if the device has poor network connectivity.";
     document.addConsoleMessage(ConsoleMessage::create(JSMessageSource, WarningMessageLevel, message));
     WTFLogAlways("%s", message.utf8().data());
 }
 
 bool shouldDisallowFetchForMainFrameScript(const ResourceRequest& request, FetchRequest::DeferOption defer, Document& document)
 {
     // Only scripts inserted via document.write are candidates for having their
     // fetch disallowed.
     if (!document.isInDocumentWrite())
         return false;

[Yoav Weiss, 2016/09/04 05:11:43]

I'm new to this code, but won't that prevent scripts from running even if we
already have them in HTTP cache or SW cache? Are we betting on the fact that
document.written scripts are probably dynamic?

[jkarlin, 2016/09/06 11:58:45]

If shouldDisallowFetchForMainFrameScript returns true, then we do check the
cache but don't go to the network.

 
     if (!document.settings())
         return false;
 
     if (!document.frame())
         return false;
 
     // Only block synchronously loaded (parser blocking) scripts.
     if (defer != FetchRequest::NoDefer)
         return false;
@@ skipping 31 matching lines @@
         // where a script was blocked could be indicative of a page break.
         document.loader()->didObserveLoadingBehavior(WebLoadingBehaviorFlag::WebLoadingBehaviorDocumentWriteBlockReload);
         return false;
     }
 
     // Add the metadata that this page has scripts inserted via document.write
     // that are eligible for blocking. Note that if there are multiple scripts
     // the flag will be conveyed to the browser process only once.
     document.loader()->didObserveLoadingBehavior(WebLoadingBehaviorFlag::WebLoadingBehaviorDocumentWriteBlock);
 
-    const bool isSlowConnection = networkStateNotifier().connectionType() == WebConnectionTypeCellular2G;
-    const bool disallowFetch = document.settings()->disallowFetchForDocWrittenScriptsInMainFrame() || (document.settings()->disallowFetchForDocWrittenScriptsInMainFrameOnSlowConnections() && isSlowConnection);
+    const bool is2G = networkStateNotifier().connectionType() == WebConnectionTypeCellular2G;
+    WebEffectiveConnectionType effectiveConnection = document.frame()->loader().client()->getEffectiveConnectionType();
+    const bool is2GOrLike2G = is2G || effectiveConnection == WebEffectiveConnectionType::Type2G || effectiveConnection == WebEffectiveConnectionType::TypeSlow2G;

[Yoav Weiss, 2016/09/04 05:11:43]

Would it have been better to change the WebEffectiveConnectionType so that
Unknown is last, and we could define `<Type2G` here?

[jkarlin, 2016/09/06 11:58:45]

I don't think a strict ordering on the enum makes much sense. Connection types
will come and go over time and will be of various speeds. Once they're used by
histograms they can only be appended to.

That said, I do think a "isConnectionTypeSlow()" that uses a switch to make sure
we've addressed all of the connection types would be better.

 
-    return disallowFetch;
+    return document.settings()->disallowFetchForDocWrittenScriptsInMainFrame() || (document.settings()->disallowFetchForDocWrittenScriptsInMainFrameOnSlowConnections() && is2G) || (RuntimeEnabledFeatures::blockDocWriteIfEffectively2GEnabled() && is2GOrLike2G);
 }
 
 } // namespace
 
 FrameFetchContext::FrameFetchContext(DocumentLoader* loader, Document* document)
     : m_document(document)
     , m_documentLoader(loader)
 {
     ASSERT(frame());
 }
@@ skipping 668 matching lines @@
 }
 
 DEFINE_TRACE(FrameFetchContext)
 {
     visitor->trace(m_document);
     visitor->trace(m_documentLoader);
     FetchContext::trace(visitor);
 }
 
 } // namespace blink