Always assume that non-auto 'columns' might create a multicol.

We can't really tell for sure, before generating the layout box tree,
whether an element is going to create a multicol container or not (which
isn't surprising, but somehow I thought it was worth a try).

This is a partial revert of CL:3320292. This part isn't really important
anymore, because CL:3323072 works around the same issue, and also
because LayoutNGBlockFragmentation is now enabled for testing.

Note that the new test would only crash if LayoutNGBlockFragmentation is
disabled (but not anymore).

Bug: 1278531
Change-Id: I80e041a076541636be7c4a7e44c0ac91805efba2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3329901
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#951029}

content/browser/accessibility/dump_accessibility_tree_browsertest.cc

@@ -166,6 +166,26 @@ class DumpAccessibilityTreeWithoutLayoutNGTest
   }
 };
 
+class DumpAccessibilityTreeWithLayoutNGBlockFragmentationTest
+    : public DumpAccessibilityTreeTest {
+ public:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    DumpAccessibilityTreeTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitchASCII(switches::kEnableBlinkFeatures,
+                                    "LayoutNGBlockFragmentation");
+  }
+};
+
+class DumpAccessibilityTreeWithoutLayoutNGBlockFragmentationTest
+    : public DumpAccessibilityTreeTest {
+ public:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    DumpAccessibilityTreeTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitchASCII(switches::kDisableBlinkFeatures,
+                                    "LayoutNGBlockFragmentation");
+  }
+};
+
 // Parameterize the tests so that each test-pass is run independently.
 struct DumpAccessibilityTreeTestPassToString {
   std::string operator()(
@@ -192,6 +212,18 @@ INSTANTIATE_TEST_SUITE_P(
     ::testing::ValuesIn(DumpAccessibilityTestHelper::TreeTestPasses()),
     DumpAccessibilityTreeTestPassToString());
 
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    DumpAccessibilityTreeWithLayoutNGBlockFragmentationTest,
+    ::testing::ValuesIn(DumpAccessibilityTestHelper::TreeTestPasses()),
+    DumpAccessibilityTreeTestPassToString());
+
+INSTANTIATE_TEST_SUITE_P(
+    All,
+    DumpAccessibilityTreeWithoutLayoutNGBlockFragmentationTest,
+    ::testing::ValuesIn(DumpAccessibilityTestHelper::TreeTestPasses()),
+    DumpAccessibilityTreeTestPassToString());
+
 IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityCSSAltText) {
   RunCSSTest(FILE_PATH_LITERAL("alt-text.html"));
 }
@@ -291,10 +323,18 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
   RunCSSTest(FILE_PATH_LITERAL("marker-hyphens.html"));
 }
 
-IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest, AccessibilityCSSMarkerCrash) {
+IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeWithLayoutNGBlockFragmentationTest,
+                       AccessibilityCSSMarkerCrash) {
   RunCSSTest(FILE_PATH_LITERAL("marker-crash.html"));
 }
 
+IN_PROC_BROWSER_TEST_P(
+    DumpAccessibilityTreeWithoutLayoutNGBlockFragmentationTest,
+    AccessibilityCSSMarkerCrash) {
+  RunCSSTest(
+      FILE_PATH_LITERAL("marker-crash-without-layout-ng-block-frag.html"));
+}
+
 IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
                        AccessibilityCSSTextOverflowEllipsis) {
   RunCSSTest(FILE_PATH_LITERAL("text-overflow-ellipsis.html"));

content/test/data/accessibility/css/marker-crash-without-layout-ng-block-frag-expected-blink.txt

@@ -0,0 +1,7 @@
+rootWebArea
+++genericContainer ignored
+++++genericContainer ignored
+++++++group
+++++++++listMarker name='%E2%80%A2 '
+++++++genericContainer ignored
+++++++++genericContainer ignored

content/test/data/accessibility/css/marker-crash-without-layout-ng-block-frag.html

@@ -0,0 +1,7 @@
+<!-- Avoid a crash caused by adding pseudo element content in two places.
+     See AXNodeObject::CanAddLayoutChild(). https://crbug.com/1172038 -->
+<style>
+span:before { display: inherit; content: ""; -webkit-column-count: 1; }
+</style>
+<span role="group" style="display: list-item"></span>
+<span class="empty"></span>

third_party/blink/renderer/core/dom/element.cc

@@ -355,14 +355,6 @@ bool DefinitelyNewFormattingContext(const Node& node,
   return false;
 }
 
-inline bool WillCreateMulticolContainer(const ComputedStyle& style) {
-  // Note that we depend on this returning the truth, i.e. no false positives,
-  // and no false negatives (which is different from the rest of the legacy
-  // fallback detection machinery, where false positives are generally
-  // acceptable). This is an honest attempt to achieve that.
-  return style.SpecifiesColumns() && style.IsDisplayBlockContainer();
-}
-
 inline bool NeedsLegacyBlockFragmentation(const Element& element,
                                           const ComputedStyle& style) {
   if (!style.InsideFragmentationContextWithNondeterministicEngine())
@@ -417,7 +409,7 @@ bool CalculateStyleShouldForceLegacyLayout(const Element& element,
   if (!RuntimeEnabledFeatures::LayoutNGBlockFragmentationEnabled()) {
     // Disable NG for the entire subtree if we're establishing a multicol
     // container.
-    if (WillCreateMulticolContainer(style)) {
+    if (style.SpecifiesColumns()) {
       UseCounter::Count(document, WebFeature::kLegacyLayoutByMultiCol);
       return true;
     }

third_party/blink/web_tests/external/wpt/css/css-multicol/img-alt-as-multicol-crash.html

@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1278531">
+<img alt="BOOM" style="columns:2;">