Fix some crashes in StylableSelect HTML parser

This patch forbids nested <button>s and <datalist>s to avoid setting
kInSelectMode while we are technically still in a <button> or
<datalist>.

This patch also replaces a call to SetInsertionMode() with
ResetInsertionModeAppropriately() and moves it after processing the end
tag in order to avoid ending up in kInSelectMode when there is not
actually an open <select> tag.

The clusterfuzz crash didn't have a minimal repro but I manually
verified that the crash does not repro with this patch.

Bug: 1511354
Fixed: 1519396
Change-Id: I4358973b36925ea0862f4050f27f804d2676288c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5254124
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1255770}

third_party/blink/renderer/core/html/parser/html_tree_builder.cc

@@ -1476,6 +1476,13 @@ void HTMLTreeBuilder::ProcessStartTag(AtomicHTMLToken* token) {
           ProcessEndTag(&end_select);
           break;
         }
+        case HTMLTag::kButton:
+        case HTMLTag::kDatalist:
+          // Don't allow nesting of buttons or datalists in this mode because
+          // when we end these tags we go back to kInSelectMode.
+          // TODO(crbug.com/1511354): Try removing this when kButton and
+          // kDatalist are handled in ResetInsertionModeAppropriately
+          break;
         default:
           ProcessStartTagForInBody(token);
           break;
@@ -1836,6 +1843,9 @@ void HTMLTreeBuilder::ResetInsertionModeAppropriately() {
             }
           }
           return SetInsertionMode(kInSelectMode);
+        // TODO(crbug.com/1511354): Consider adding cases for kButton and
+        // kDatalist here to reset the insertion mode to kInButtonInSelectMode
+        // and kInDatalistInSelectMode
         case HTMLTag::kTd:
         case HTMLTag::kTh:
           return SetInsertionMode(kInCellMode);
@@ -2419,11 +2429,7 @@ void HTMLTreeBuilder::ProcessEndTag(AtomicHTMLToken* token) {
     case kInButtonInSelectMode:
       CHECK(RuntimeEnabledFeatures::StylableSelectEnabled());
       switch (tag) {
-        case HTMLTag::kButton:
-          SetInsertionMode(kInSelectMode);
-          break;
         case HTMLTag::kSelect:
-          SetInsertionMode(kInSelectMode);
           tree_.OpenElements()->PopUntilPopped(HTMLTag::kSelect);
           ResetInsertionModeAppropriately();
           return;
@@ -2433,12 +2439,12 @@ void HTMLTreeBuilder::ProcessEndTag(AtomicHTMLToken* token) {
       // TODO(crbug.com/1511354): We need to review which end tags cause which
       // elements to close once there's a reviewed HTML PR for styleable select.
       ProcessEndTagForInBody(token);
+      if (tag == HTMLTag::kButton) {
+        ResetInsertionModeAppropriately();
+      }
       return;
     case kInDatalistInSelectMode:
       switch (tag) {
-        case HTMLTag::kDatalist:
-          SetInsertionMode(kInSelectMode);
-          break;
         case HTMLTag::kSelect:
           SetInsertionMode(kInSelectMode);
           tree_.OpenElements()->PopUntilPopped(HTMLTag::kSelect);
@@ -2448,6 +2454,9 @@ void HTMLTreeBuilder::ProcessEndTag(AtomicHTMLToken* token) {
           break;
       }
       ProcessEndTagForInBody(token);
+      if (tag == HTMLTag::kDatalist) {
+        ResetInsertionModeAppropriately();
+      }
       return;
     case kInSelectInTableMode:
       switch (tag) {

third_party/blink/web_tests/external/wpt/html/semantics/forms/the-select-element/select-parsing.tentative.html

@@ -17,36 +17,34 @@
 </select>
 
 <select id=s2>
-  <div>div 1</div>
-  <button>
-    <span>level 1</span>
-    <button>
-      <span>level 2</span>
-    </button>
-  </button>
-  <div>div 2</div>
-</select>
-
-<select id=s3>
   <button>button
 </select>
 
-<select id=s4>
+<select id=s3>
   <datalist>datalist
 </select>
 
-<select id=s5>
+<select id=s4>
   <button>
     <select></select>
   </button>
 </select>
 
-<select id=s6>
+<select id=s5>
   <button>
     <div>
       <select>
 </select>
 
+<select id=s6>
+<button>
+<button></button>
+</button>
+<datalist>
+<datalist></datalist>
+</datalist>
+</select>
+
 <div id=afterlast>
   keep this div after the last test case
 </div>
@@ -70,45 +68,44 @@ test(() => {
 test(() => {
   assert_equals(document.getElementById('s2').parentNode, document.body);
   assert_equals(document.getElementById('s2').innerHTML, `
-  div 1
-  <button>
-    <span>level 1</span>
-    </button><button>
-      <span>level 2</span>
-    </button>
-  \n  div 2
-`);
-}, 'Nested <button>s in <select> should be flattened out.');
-
-test(() => {
-  assert_equals(document.getElementById('s3').parentNode, document.body);
-  assert_equals(document.getElementById('s3').innerHTML, `
   <button>button
 </button>`);
 }, '</select> should close <button>.');
 
 test(() => {
-  assert_equals(document.getElementById('s4').parentNode, document.body);
-  assert_equals(document.getElementById('s4').innerHTML, `
+  assert_equals(document.getElementById('s3').parentNode, document.body);
+  assert_equals(document.getElementById('s3').innerHTML, `
   <datalist>datalist
 </datalist>`);
 }, '</select> should close <datalist>.');
 
 test(() => {
-  assert_equals(document.getElementById('s5').parentNode, document.body);
-  assert_equals(document.getElementById('s5').innerHTML, `
+  assert_equals(document.getElementById('s4').parentNode, document.body);
+  assert_equals(document.getElementById('s4').innerHTML, `
   <button>
     </button>`);
 }, '<select> in <button> in <select> should remove inner <select>.');
 
 test(() => {
-  assert_equals(document.getElementById('s6').parentNode, document.body);
-  assert_equals(document.getElementById('s6').innerHTML, `
+  assert_equals(document.getElementById('s5').parentNode, document.body);
+  assert_equals(document.getElementById('s5').innerHTML, `
   <button>
     <div>
       </div></button>`);
 }, '<select> in <select><button><div> should remove inner <select>.');
 
+test(() => {
+  assert_equals(document.getElementById('s6').parentNode, document.body);
+  assert_equals(document.getElementById('s6').innerHTML, `
+<button>
+</button>
+
+<datalist>
+</datalist>
+
+`);
+}, 'Nested <button>s or <datalist>s in <select> should be dropped.');
+
 test(() => {
   assert_equals(document.getElementById('afterlast').parentNode, document.body);
 }, 'The last test should not leave any tags open after parsing.');

third_party/blink/web_tests/virtual/stylable-select-disabled/external/wpt/html/semantics/forms/the-select-element/select-parsing.tentative-expected.txt

@@ -1,8 +1,6 @@
 This is a testharness.js-based test.
 [FAIL] <button>s and <datalist>s should be allowed in <select>.
   assert_equals: expected "\\n  div 1\\n  <button>button</button>\\n  div 2\\n  <datalist>\\n    <option>option</option>\\n  </datalist>\\n  div 3\\n" but got "\\n  div 1\\n  button\\n  div 2\\n  \\n    <option>option</option>\\n  \\n  div 3\\n"
-[FAIL] Nested <button>s in <select> should be flattened out.
-  assert_equals: expected "\\n  div 1\\n  <button>\\n    <span>level 1</span>\\n    </button><button>\\n      <span>level 2</span>\\n    </button>\\n  \\n  div 2\\n" but got "\\n  div 1\\n  \\n    level 1\\n    \\n      level 2\\n    \\n  \\n  div 2\\n"
 [FAIL] </select> should close <button>.
   assert_equals: expected "\\n  <button>button\\n</button>" but got "\\n  button\\n"
 [FAIL] </select> should close <datalist>.
@@ -11,5 +9,7 @@ This is a testharness.js-based test.
   assert_equals: expected "\\n  <button>\\n    </button>" but got "\\n  \\n    "
 [FAIL] <select> in <select><button><div> should remove inner <select>.
   assert_equals: expected "\\n  <button>\\n    <div>\\n      </div></button>" but got "\\n  \\n    \\n      "
+[FAIL] Nested <button>s or <datalist>s in <select> should be dropped.
+  assert_equals: expected "\\n<button>\\n</button>\\n\\n<datalist>\\n</datalist>\\n\\n" but got "\\n\\n\\n\\n\\n\\n\\n"
 Harness: the test ran to completion.