Commit 25571758 authored by Manuel Rego Casasnovas's avatar Manuel Rego Casasnovas Committed by Commit Bot

Fix CSSFlexibleBox use counter

Until now CSSFlexibleBox use counter was counting
not only "display: flex|inline-flex" elements,
but also buttons or any other classes that inherit
from LayoutFlexbibleBox:
That's an implementation detail and we shouldn't be interested
in counting those cases as flexbox usage.

The patch moves the counter to LayoutObject::CreateObject()
so we actually check the display value.
The patch also moves the grid layout counter,
despite nobody is inheriting from LayoutGrid now,
a similar issue could happen in the future if someone does it.

New unit tests are added to verify that things are working
as expected now.

Change-Id: I716870a666a4167d19153667bc80f68a3ba0d7e3
Reviewed-on: https://chromium-review.googlesource.com/c/1273107
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598791}
parent ea66e78f
...@@ -321,6 +321,46 @@ TEST_F(UseCounterTest, CSSGridLayoutPercentageRowIndefiniteHeight) { ...@@ -321,6 +321,46 @@ TEST_F(UseCounterTest, CSSGridLayoutPercentageRowIndefiniteHeight) {
EXPECT_TRUE(UseCounter::IsCounted(document, feature)); EXPECT_TRUE(UseCounter::IsCounted(document, feature));
} }
TEST_F(UseCounterTest, CSSFlexibleBox) {
std::unique_ptr<DummyPageHolder> dummy_page_holder =
DummyPageHolder::Create(IntSize(800, 600));
Page::InsertOrdinaryPageForTesting(&dummy_page_holder->GetPage());
Document& document = dummy_page_holder->GetDocument();
WebFeature feature = WebFeature::kCSSFlexibleBox;
EXPECT_FALSE(UseCounter::IsCounted(document, feature));
document.documentElement()->SetInnerHTMLFromString(
"<div style='display: flex;'>flexbox</div>");
document.View()->UpdateAllLifecyclePhases();
EXPECT_TRUE(UseCounter::IsCounted(document, feature));
}
TEST_F(UseCounterTest, CSSFlexibleBoxInline) {
std::unique_ptr<DummyPageHolder> dummy_page_holder =
DummyPageHolder::Create(IntSize(800, 600));
Page::InsertOrdinaryPageForTesting(&dummy_page_holder->GetPage());
Document& document = dummy_page_holder->GetDocument();
WebFeature feature = WebFeature::kCSSFlexibleBox;
EXPECT_FALSE(UseCounter::IsCounted(document, feature));
document.documentElement()->SetInnerHTMLFromString(
"<div style='display: inline-flex;'>flexbox</div>");
document.View()->UpdateAllLifecyclePhases();
EXPECT_TRUE(UseCounter::IsCounted(document, feature));
}
TEST_F(UseCounterTest, CSSFlexibleBoxButton) {
// LayoutButton is a subclass of LayoutFlexibleBox, however we don't want it
// to be counted as usage of flexboxes as it's an implementation detail.
std::unique_ptr<DummyPageHolder> dummy_page_holder =
DummyPageHolder::Create(IntSize(800, 600));
Page::InsertOrdinaryPageForTesting(&dummy_page_holder->GetPage());
Document& document = dummy_page_holder->GetDocument();
WebFeature feature = WebFeature::kCSSFlexibleBox;
EXPECT_FALSE(UseCounter::IsCounted(document, feature));
document.documentElement()->SetInnerHTMLFromString("<button>button</button>");
document.View()->UpdateAllLifecyclePhases();
EXPECT_FALSE(UseCounter::IsCounted(document, feature));
}
class DeprecationTest : public testing::Test { class DeprecationTest : public testing::Test {
public: public:
DeprecationTest() DeprecationTest()
......
...@@ -60,8 +60,6 @@ LayoutFlexibleBox::LayoutFlexibleBox(Element* element) ...@@ -60,8 +60,6 @@ LayoutFlexibleBox::LayoutFlexibleBox(Element* element)
has_definite_height_(SizeDefiniteness::kUnknown), has_definite_height_(SizeDefiniteness::kUnknown),
in_layout_(false) { in_layout_(false) {
DCHECK(!ChildrenInline()); DCHECK(!ChildrenInline());
if (!IsAnonymous())
UseCounter::Count(GetDocument(), WebFeature::kCSSFlexibleBox);
} }
LayoutFlexibleBox::~LayoutFlexibleBox() = default; LayoutFlexibleBox::~LayoutFlexibleBox() = default;
......
...@@ -49,8 +49,6 @@ LayoutGrid::LayoutGrid(Element* element) ...@@ -49,8 +49,6 @@ LayoutGrid::LayoutGrid(Element* element)
grid_(Grid::Create(this)), grid_(Grid::Create(this)),
track_sizing_algorithm_(this, *grid_) { track_sizing_algorithm_(this, *grid_) {
DCHECK(!ChildrenInline()); DCHECK(!ChildrenInline());
if (!IsAnonymous())
UseCounter::Count(GetDocument(), WebFeature::kCSSGridLayout);
} }
LayoutGrid::~LayoutGrid() = default; LayoutGrid::~LayoutGrid() = default;
......
...@@ -268,9 +268,11 @@ LayoutObject* LayoutObject::CreateObject(Element* element, ...@@ -268,9 +268,11 @@ LayoutObject* LayoutObject::CreateObject(Element* element,
return new LayoutDeprecatedFlexibleBox(*element); return new LayoutDeprecatedFlexibleBox(*element);
case EDisplay::kFlex: case EDisplay::kFlex:
case EDisplay::kInlineFlex: case EDisplay::kInlineFlex:
UseCounter::Count(element->GetDocument(), WebFeature::kCSSFlexibleBox);
return LayoutObjectFactory::CreateFlexibleBox(*element, style); return LayoutObjectFactory::CreateFlexibleBox(*element, style);
case EDisplay::kGrid: case EDisplay::kGrid:
case EDisplay::kInlineGrid: case EDisplay::kInlineGrid:
UseCounter::Count(element->GetDocument(), WebFeature::kCSSGridLayout);
return new LayoutGrid(element); return new LayoutGrid(element);
case EDisplay::kLayoutCustom: case EDisplay::kLayoutCustom:
case EDisplay::kInlineLayoutCustom: case EDisplay::kInlineLayoutCustom:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment