Implement new <link disabled> handling logic

Previously, Chromium had intermittent behavior with respect to the
"disabled" attribute:
- <link id=foo rel="stylesheet" disabled> would not show up in
  document.styleSheets.
- foo.disabled=false; foo.disabled=true; would cause it to appear
  (and remain) in document.styleSheets.
- <link rel="alternate stylesheet"> cannot be enabled, except by
  disabling and re-enabling it.
- When disabled, link.ownerNode was not null

The above issues are being resolved with this CL.

See the spec discussion and PR here:
 https://github.com/whatwg/html/issues/3840
 https://github.com/whatwg/html/pull/4519

And the ChromeStatus and I2S here:
 https://chromestatus.com/feature/5110851973414912
 https://groups.google.com/a/chromium.org/d/msg/blink-dev/3izM5vVo98U/iDLjz_TwAgAJ=

WPT tests are located here:
 https://wpt.fyi/results/css/cssom?label=master&label=experimental&aligned&q=htmllinkelement-disabled
and they show that Firefox already implements the new behavior,
while WebKit does not. With this CL, all 7 tests are now green.

Bug: 695984, 1087043
Change-Id: Ic6d2c8e901a72885b9f4806c616adb95e019cf09
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2216701Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774658}

content/child/runtime_features.cc

@@ -365,6 +365,8 @@ void SetRuntimeFeaturesFromChromiumFeatures() {
           {"LayoutNGFlexBox", blink::features::kFlexNG, kUseFeatureState},
           {"LegacyWindowsDWriteFontFallback",
            features::kLegacyWindowsDWriteFontFallback, kUseFeatureState},
+          {"LinkDisabledNewSpecBehavior",
+           blink::features::kLinkDisabledNewSpecBehavior, kUseFeatureState},
           {"OriginPolicy", features::kOriginPolicy, kUseFeatureState},
           {"OriginIsolationHeader", features::kOriginIsolationHeader,
            kUseFeatureState},

third_party/blink/common/features.cc

@@ -478,6 +478,12 @@ const base::Feature kBlockFlowHandlesWebkitLineClamp{
 const base::Feature kBlockHTMLParserOnStyleSheets{
     "BlockHTMLParserOnStyleSheets", base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Kill switch for the new <link disabled> behavior.
+// TODO(crbug.com/1087043): Remove this once the feature has
+// landed and no compat issues are reported.
+const base::Feature kLinkDisabledNewSpecBehavior{
+    "LinkDisabledNewSpecBehavior", base::FEATURE_ENABLED_BY_DEFAULT};
+
 // Slightly delays rendering if there are fonts being preloaded, so that
 // they don't miss the first paint if they can be loaded fast enough (e.g.,
 // from the disk cache)

third_party/blink/public/common/features.h

@@ -156,6 +156,8 @@ BLINK_COMMON_EXPORT extern const base::Feature kBlockFlowHandlesWebkitLineClamp;
 
 BLINK_COMMON_EXPORT extern const base::Feature kBlockHTMLParserOnStyleSheets;
 
+BLINK_COMMON_EXPORT extern const base::Feature kLinkDisabledNewSpecBehavior;
+
 BLINK_COMMON_EXPORT extern const base::Feature kFontPreloadingDelaysRendering;
 BLINK_COMMON_EXPORT extern const base::FeatureParam<int>
     kFontPreloadingDelaysRenderingParam;

third_party/blink/renderer/core/html/html_link_element.cc

@@ -145,8 +145,20 @@ void HTMLLinkElement::ParseAttribute(
     UseCounter::Count(GetDocument(), WebFeature::kHTMLLinkElementDisabled);
     if (params.reason == AttributeModificationReason::kByParser)
       UseCounter::Count(GetDocument(), WebFeature::kHTMLLinkElementDisabledByParser);
-    if (LinkStyle* link = GetLinkStyle())
+    // TODO(crbug.com/1087043): Remove this if() condition once the feature has
+    // landed and no compat issues are reported.
+    if (RuntimeEnabledFeatures::LinkDisabledNewSpecBehaviorEnabled(
+            &GetDocument())) {
+      LinkStyle* link = GetLinkStyle();
+      if (!link) {
+        link = MakeGarbageCollected<LinkStyle>(this);
+        link_ = link;
+      }
       link->SetDisabledState(!value.IsNull());
+    } else {
+      if (LinkStyle* link = GetLinkStyle())
+        link->SetDisabledState(!value.IsNull());
+    }
   } else {
     if (name == html_names::kTitleAttr) {
       if (LinkStyle* link = GetLinkStyle())

third_party/blink/renderer/core/html/html_link_element.h

@@ -110,7 +110,14 @@ class CORE_EXPORT HTMLLinkElement final : public HTMLElement,
                       FetchParameters::DeferOption,
                       ResourceClient*);
   bool IsAlternate() const {
-    return GetLinkStyle()->IsUnset() && rel_attribute_.IsAlternate();
+    // TODO(crbug.com/1087043): Remove this if() condition once the feature has
+    // landed and no compat issues are reported.
+    bool not_explicitly_enabled =
+        !GetLinkStyle()->IsExplicitlyEnabled() ||
+        !RuntimeEnabledFeatures::LinkDisabledNewSpecBehaviorEnabled(
+            &GetDocument());
+    return GetLinkStyle()->IsUnset() && rel_attribute_.IsAlternate() &&
+           not_explicitly_enabled;
   }
   bool ShouldProcessStyle() {
     return LinkResourceToProcess() && GetLinkStyle();

third_party/blink/renderer/core/html/link_style.cc

@@ -202,6 +202,10 @@ void LinkStyle::RemovePendingSheet() {
 void LinkStyle::SetDisabledState(bool disabled) {
   LinkStyle::DisabledState old_disabled_state = disabled_state_;
   disabled_state_ = disabled ? kDisabled : kEnabledViaScript;
+  // Whenever the disabled attribute is removed, set the link element's
+  // explicitly enabled attribute to true.
+  if (!disabled)
+    explicitly_enabled_ = true;
   if (old_disabled_state == disabled_state_)
     return;
 
@@ -231,8 +235,19 @@ void LinkStyle::SetDisabledState(bool disabled) {
   }
 
   if (sheet_) {
-    sheet_->setDisabled(disabled);
-    return;
+    // TODO(crbug.com/1087043): Remove this if() condition once the feature has
+    // landed and no compat issues are reported.
+    if (RuntimeEnabledFeatures::LinkDisabledNewSpecBehaviorEnabled(
+            &GetDocument())) {
+      DCHECK(disabled)
+          << "If link is being enabled, sheet_ shouldn't exist yet";
+      ClearSheet();
+      GetDocument().GetStyleEngine().SetNeedsActiveStyleUpdate(
+          owner_->GetTreeScope());
+      return;
+    } else {
+      sheet_->setDisabled(disabled);
+    }
   }
 
   if (disabled_state_ == kEnabledViaScript && owner_->ShouldProcessStyle())

third_party/blink/renderer/core/html/link_style.h

@@ -54,6 +54,8 @@ class LinkStyle final : public LinkResource, ResourceClient {
   }
   bool IsUnset() const { return disabled_state_ == kUnset; }
 
+  bool IsExplicitlyEnabled() const { return explicitly_enabled_; }
+
   CSSStyleSheet* Sheet() const { return sheet_.Get(); }
 
  private:
@@ -76,6 +78,7 @@ class LinkStyle final : public LinkResource, ResourceClient {
   DisabledState disabled_state_;
   PendingSheetType pending_sheet_type_;
   StyleEngineContext style_engine_context_;
+  bool explicitly_enabled_;
   bool loading_;
   bool fired_load_;
   bool loaded_sheet_;

third_party/blink/renderer/platform/runtime_enabled_features.json5

@@ -1000,6 +1000,12 @@
       name: "LegacyWindowsDWriteFontFallback",
       // Enabled by features::kLegacyWindowsDWriteFontFallback;
     },
+    {
+      // TODO(crbug.com/1087043): Remove this once the feature has
+      // landed and no compat issues are reported.
+      name: "LinkDisabledNewSpecBehavior",
+      status: "stable",
+    },
     {
       name:"ManualSlotting",
       status:"experimental",

third_party/blink/web_tests/VirtualTestSuites

@@ -766,6 +766,11 @@
     "bases": [ "inspector-protocol/media" ],
     "args": ["--enable-features=MediaInspectorLogging"]
   },
+  {
+    "prefix": "link-disabled-old-behavior",
+    "bases": [ "external/wpt/css/cssom" ],
+    "args": ["--disable-features=LinkDisabledNewSpecBehavior"]
+  },
   {
     "prefix": "media-capture-image-ptz",
     "bases": [ "external/wpt/mediacapture-image" ],

third_party/blink/web_tests/cssom/stylesheetlist-01.html

@@ -14,7 +14,7 @@
       var colorTest = async_test("Check that no stylesheets are applied when disabled");
       var testObj = new Object;
       for (var i=0; i<5; i++) {
-        testObj[i] = async_test("#s"+(i+1)+" accessible through document.styleSheets after disabling");
+        testObj[i] = async_test(`#s${i+1}${(i<3) ? ' NOT' : ''} accessible through document.styleSheets after disabling`);
       }
 
       onload = function(){
@@ -38,13 +38,18 @@
         colorTest.done();
 
         count = document.styleSheets.length;
+        assert_equals(count, 2); // Just the <style> nodes
         for (i=0; i<count; i++)
           testObj[document.styleSheets[i].ownerNode.id] = true;
 
         for (i=0; i<5; i++) {
           testObj[i].step(function(){
-            assert_true(testObj["s"+(i+1)]);
-          }, "Check if #s"+(i+1)+" is present in document.styleSheets after it is disabled");
+            if (i<3) {
+              assert_true(!testObj["s"+(i+1)]);
+            } else {
+              assert_true(testObj["s"+(i+1)]);
+            }
+          }, "Check if #s"+(i+1)+" is present in document.styleSheets for <style> but not <link> after it is disabled");
           testObj[i].done();
         }
       };

third_party/blink/web_tests/external/wpt/css/cssom/HTMLLinkElement-disabled-001.tentative-expected.txt

-This is a testharness.js-based test.
-PASS <link disabled> prevents the stylesheet from being in document.styleSheets (from parser)
-FAIL HTMLLinkElement.disabled reflects the <link disabled> attribute, and behaves consistently assert_equals: expected null but got Element node <link title="alt" rel="stylesheet" href="data:text/css,ht...
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/css/cssom/HTMLLinkElement-disabled-002.tentative-expected.txt

-This is a testharness.js-based test.
-FAIL HTMLLinkElement.disabled reflects the <link disabled> attribute, and behaves consistently, when the sheet is an alternate assert_equals: expected null but got Element node <link title="alt" rel="alternate stylesheet" href="data:t...
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/css/cssom/HTMLLinkElement-disabled-007.tentative-expected.txt

-This is a testharness.js-based test.
-FAIL HTMLLinkElement.disabled setter sets the explicitly enabled state if toggled back and forth. assert_equals: expected "rgb(0, 128, 0)" but got "rgba(0, 0, 0, 0)"
-Harness: the test ran to completion.
-

third_party/blink/web_tests/fast/dom/HTMLLinkElement/disabled-attribute-expected.txt

@@ -4,14 +4,12 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 PASS test.disabled is false
 PASS test.disabled is true
-PASS test.sheet.disabled is true
-PASS test.sheet.disabled is false
-FAIL test.disabled should be false. Was true.
+PASS test.sheet is null
 
 
 PASS test_nostyle.sheet is null
 PASS test_nostyle.disabled is false
-FAIL test_nostyle.disabled should be false. Was true.
+PASS test_nostyle.disabled is true
 PASS successfullyParsed is true
 
 TEST COMPLETE

third_party/blink/web_tests/fast/dom/HTMLLinkElement/disabled-attribute.html

@@ -18,13 +18,8 @@ shouldBe('test.disabled', 'false');
 // set value to true
 test.disabled = true;
 shouldBe('test.disabled', 'true');
-// Updated value should be reflected in style.sheet.disabled, 'true' in this case
-shouldBe('test.sheet.disabled', 'true');
-// now set style.sheet.disabled to false
-test.sheet.disabled = false;
-shouldBe('test.sheet.disabled', 'false');
-// Updated value should be refectled in test.disabled
-shouldBe('test.disabled', 'false');
+// Disabled <link> should have a null sheet
+shouldBe('test.sheet', 'null');
 debug('<br>');
 // verify link.sheet is null since it does not contain an href attribute
 shouldBe('test_nostyle.sheet', 'null');
@@ -32,8 +27,7 @@ shouldBe('test_nostyle.sheet', 'null');
 shouldBe('test_nostyle.disabled', 'false');
 // setting it to true
 test_nostyle.disabled = true;
-// should do nothing on setting.
-shouldBe('test_nostyle.disabled', 'false');
+shouldBe('test_nostyle.disabled', 'true');
 
 
 </script>

third_party/blink/web_tests/virtual/link-disabled-old-behavior/README.md

+This is a virtual test suite for the *old* <link disabled> behavior,
+to ensure it respects the Feature flag, in case this behavior needs to be
+disabled via Finch.
+
+Flag: --disable-features=LinkDisabledNewSpecBehavior
+Bug: crbug.com/1087043