Don't trigger a link resource fetch on media change.

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 174 matching lines @@
     } else if (name == typeAttr) {
         m_type = value;
         process();
     } else if (name == asAttr) {
         m_as = value;
         process();
     } else if (name == sizesAttr) {
         m_sizes->setValue(value);
     } else if (name == mediaAttr) {
         m_media = value.lower();
-        process();
+        if (m_link)
+            m_link->mediaChanged();

[Yoav Weiss, 2016/03/14 15:23:51]

Seems to me that we would want "process()" not to trigger a second download if
it's not necessary rather than (or on top of) handling this `media` specific
case.

Maybe "process()" can check current URL and see if it is changed since it was
fetched? (and perhaps special-case media changes as to trigger mediaChanged
rather than a download? Not sure it's necessary)

Long term, I think we may want to move the loading code from here and into
loader (maybe split LinkLoader into LinkStyleLoader, LinkPreloadLoader, etc?)

[rune, 2016/04/07 07:56:07]

Yes, mediaChanged() is necessary to trigger a style update. Having process()
checking attributes without given the knowledge of what changed sounds
cumbersome. Instead of using the process method, we could have an
attributeChanged() for each of the LinkResource types. If there is no
LinkResources, call process(). If the rel type changed in such a way that the
LinkResource type should change, dispose the old one and create a new one with
process().

This code seems broken in so many ways and needs an overhaul. A couple of
issues:

- If we create a LinkResource for one rel-type, it cannot be changed afterwards
(works in Gecko).
- If we set the any of the known attributes to the same value ten times, it will
trigger ten resource loads.

If my media specific fix is not acceptable, I'll defer this to loading-dev
people for now as I need to focus elsewhere.

     } else if (name == disabledAttr) {
         UseCounter::count(document(), UseCounter::HTMLLinkElementDisabled);
         if (LinkStyle* link = linkStyle())
             link->setDisabledState(!value.isNull());
     } else {
         if (name == titleAttr) {
             if (LinkStyle* link = linkStyle())
                 link->setSheetTitle(value);
         }
 
@@ skipping 474 matching lines @@
         if (m_sheet) {
             m_sheet->setDisabled(disabled);
             return;
         }
 
         if (m_disabledState == EnabledViaScript && m_owner->shouldProcessStyle())
             process();
     }
 }
 
+void LinkStyle::mediaChanged()
+{
+    if (m_sheet) {
+        m_sheet->setMediaQueries(MediaQuerySet::create(m_owner->media()));
+        document().modifiedStyleSheet(m_sheet.get());
+    }
+}
+
 void LinkStyle::setCrossOriginStylesheetStatus(CSSStyleSheet* sheet)
 {
     if (m_fetchFollowingCORS && resource() && !resource()->errorOccurred()) {
         // Record the security origin the CORS access check succeeded at, if cross origin.
         // Only origins that are script accessible to it may access the stylesheet's rules.
         sheet->setAllowRuleAccessFromOrigin(m_owner->document().securityOrigin());
     }
     m_fetchFollowingCORS = false;
 }
 
@@ skipping 86 matching lines @@
 }
 
 DEFINE_TRACE(LinkStyle)
 {
     visitor->trace(m_sheet);
     LinkResource::trace(visitor);
     ResourceOwner<StyleSheetResource>::trace(visitor);
 }
 
 } // namespace blink