Commit 2472878c authored by Matt Giuca's avatar Matt Giuca Committed by Commit Bot

Revert "Delete constructor for creating empty CSSStyleSheets"

This reverts commit af00b29e.

Reason for revert: Suspect for https://crbug.com/873015

I have low confidence that this is the culprit but it's my best guess.

Original change's description:
> Delete constructor for creating empty CSSStyleSheets
> 
> Currently, empty CSSStyleSheets can be constructed either with a constructor or with Document.createEmptyCSSStyleSheet.
> This CL deletes the constructor so that they can only be produced by Document.createEmptyCSSStyleSheet.
> 
> Document.createEmptyCSSStyleSheet is considered to be more desirable, as CSSStyleSheets produced by Document.createEmptyCSSStyleSheet can be tied to documents in the future. This means that their use can be limited in the documents where they were produced, resulting in higher security.
> 
> Note:
> The constructed CSSStyleSheet is not currently tied to the Document yet
> 
> Link to related comments in discussion:
> https://github.com/WICG/construct-stylesheets/issues/23#issuecomment-379180786
> https://github.com/WICG/construct-stylesheets/issues/15#issuecomment-391216056
> 
> Bug: 807560
> Change-Id: I767e15e83e1f31eb278bc81233c8b579d0f511c7
> Reviewed-on: https://chromium-review.googlesource.com/1164876
> Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
> Reviewed-by: Hayato Ito <hayato@chromium.org>
> Commit-Queue: Momoko Sumida <momon@google.com>
> Cr-Commit-Position: refs/heads/master@{#581836}

TBR=hayato@chromium.org,rakina@chromium.org,momon@google.com

Change-Id: Iea70ff4dcc3a5ecf3a806b417572fd8f88e2e58b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 807560, 873015
Reviewed-on: https://chromium-review.googlesource.com/1170462Reviewed-by: default avatarMatt Giuca <mgiuca@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582054}
parent e4238000
This is a testharness.js-based test.
Found 377 tests; 323 PASS, 54 FAIL, 0 TIMEOUT, 0 NOTRUN.
Found 377 tests; 322 PASS, 55 FAIL, 0 TIMEOUT, 0 NOTRUN.
PASS Test driver
PASS Partial interface Document: original interface defined
PASS Partial interface Window: original interface defined
......@@ -38,7 +38,7 @@ PASS StyleSheet interface: attribute parentStyleSheet
PASS StyleSheet interface: attribute title
PASS StyleSheet interface: attribute media
PASS StyleSheet interface: attribute disabled
PASS CSSStyleSheet interface: existence and properties of interface object
FAIL CSSStyleSheet interface: existence and properties of interface object assert_throws: interface object didn't throw TypeError when called as a constructor function "function () { [native code] }" did not throw
PASS CSSStyleSheet interface object length
PASS CSSStyleSheet interface object name
PASS CSSStyleSheet interface: existence and properties of interface prototype object
......
......@@ -8,6 +8,26 @@
const redStyleTexts = [".red { color: red; }", ".red + span + span { color: red; }"];
test(() => {
const sheet = new CSSStyleSheet({title: "Red", disabled: true, media: "screen, print"});
assert_equals(sheet.title, "Red");
assert_equals(sheet.ownerNode, null);
assert_equals(sheet.ownerRule, null);
assert_equals(sheet.media.length, 2);
assert_equals(sheet.media.item(0), "screen");
assert_equals(sheet.media.item(1), "print");
assert_true(sheet.disabled);
assert_equals(sheet.cssRules.length, 0);
sheet.insertRule(redStyleTexts[0]);
assert_equals(sheet.cssRules.length, 1);
assert_equals(sheet.cssRules[0].cssText, redStyleTexts[0]);
sheet.insertRule(redStyleTexts[1]);
assert_equals(sheet.cssRules.length, 2);
assert_equals(sheet.cssRules[0].cssText, redStyleTexts[1]);
}, 'Empty CSSStyleSheet can be constructed using script');
test(() => {
const sheet = document.createEmptyCSSStyleSheet({title: "Red", disabled: true, media: "screen, print"});
assert_equals(sheet.title, "Red");
......
......@@ -74,7 +74,7 @@ PASS TryAllocate('CSSRule') is 'exception'
PASS TryAllocate('CSSRuleList') is 'exception'
PASS TryAllocate('CSSStyleDeclaration') is 'exception'
PASS TryAllocate('CSSStyleRule') is 'exception'
PASS TryAllocate('CSSStyleSheet') is 'exception'
FAIL TryAllocate('CSSStyleSheet') should be exception. Was [object CSSStyleSheet].
PASS TryAllocate('DOMImplementation') is 'exception'
PASS TryAllocate('HTMLCollection') is 'exception'
PASS TryAllocate('MediaList') is 'exception'
......
......@@ -91,6 +91,11 @@ const Document* CSSStyleSheet::SingleOwnerDocument(
return nullptr;
}
CSSStyleSheet* CSSStyleSheet::Create(Document& document,
ExceptionState& exception_state) {
return CSSStyleSheet::Create(document, CSSStyleSheetInit(), exception_state);
}
CSSStyleSheet* CSSStyleSheet::Create(Document& document,
const CSSStyleSheetInit& options,
ExceptionState& exception_state) {
......
......@@ -53,6 +53,7 @@ class CORE_EXPORT CSSStyleSheet final : public StyleSheet {
public:
static const Document* SingleOwnerDocument(const CSSStyleSheet*);
static CSSStyleSheet* Create(Document&, ExceptionState&);
static CSSStyleSheet* Create(Document&,
const CSSStyleSheetInit&,
ExceptionState&);
......@@ -195,11 +196,6 @@ class CORE_EXPORT CSSStyleSheet final : public StyleSheet {
FRIEND_TEST_ALL_PREFIXES(
CSSStyleSheetTest,
CSSStyleSheetConstructionWithNonEmptyCSSStyleSheetInit);
FRIEND_TEST_ALL_PREFIXES(CSSStyleSheetTest,
CreateEmptyCSSStyleSheetWithEmptyCSSStyleSheetInit);
FRIEND_TEST_ALL_PREFIXES(
CSSStyleSheetTest,
CreateEmptyCSSStyleSheetWithNonEmptyCSSStyleSheetInit);
FRIEND_TEST_ALL_PREFIXES(
CSSStyleSheetTest,
CreateCSSStyleSheetWithEmptyCSSStyleSheetInitAndText);
......
......@@ -21,6 +21,9 @@
// https://drafts.csswg.org/cssom/#the-cssstylesheet-interface
[
ConstructorCallWith=Document,
RaisesException=Constructor,
Constructor(optional CSSStyleSheetInit options),
Exposed=Window
] interface CSSStyleSheet : StyleSheet {
readonly attribute CSSRule? ownerRule;
......
......@@ -49,41 +49,13 @@ class CSSStyleSheetTest : public PageTestBase {
TEST_F(CSSStyleSheetTest, ConstructorWithoutRuntimeFlagThrowsException) {
DummyExceptionStateForTesting exception_state;
RuntimeEnabledFeatures::SetConstructableStylesheetsEnabled(false);
EXPECT_EQ(CSSStyleSheet::Create(GetDocument(), CSSStyleSheetInit(),
exception_state),
nullptr);
EXPECT_EQ(CSSStyleSheet::Create(GetDocument(), exception_state), nullptr);
ASSERT_TRUE(exception_state.HadException());
}
TEST_F(CSSStyleSheetTest,
CSSStyleSheetConstructionWithNonEmptyCSSStyleSheetInit) {
DummyExceptionStateForTesting exception_state;
CSSStyleSheetInit init;
init.setMedia(MediaListOrString::FromString("screen, print"));
init.setTitle("test");
init.setAlternate(true);
init.setDisabled(true);
CSSStyleSheet* sheet =
CSSStyleSheet::Create(GetDocument(), init, exception_state);
ASSERT_FALSE(exception_state.HadException());
EXPECT_TRUE(sheet->href().IsNull());
EXPECT_EQ(sheet->parentStyleSheet(), nullptr);
EXPECT_EQ(sheet->ownerNode(), nullptr);
EXPECT_EQ(sheet->ownerRule(), nullptr);
EXPECT_EQ(sheet->media()->length(), 2U);
EXPECT_EQ(sheet->media()->mediaText(), init.media().GetAsString());
EXPECT_EQ(sheet->title(), init.title());
EXPECT_TRUE(sheet->AlternateFromConstructor());
EXPECT_TRUE(sheet->disabled());
EXPECT_EQ(sheet->cssRules(exception_state)->length(), 0U);
ASSERT_FALSE(exception_state.HadException());
}
TEST_F(CSSStyleSheetTest, CreateEmptyCSSStyleSheetWithEmptyCSSStyleSheetInit) {
V8TestingScope scope;
TEST_F(CSSStyleSheetTest, CSSStyleSheetConstructionWithEmptyCSSStyleSheetInit) {
DummyExceptionStateForTesting exception_state;
CSSStyleSheet* sheet = GetDocument().createEmptyCSSStyleSheet(
scope.GetScriptState(), CSSStyleSheetInit(), exception_state);
CSSStyleSheet* sheet = CSSStyleSheet::Create(GetDocument(), exception_state);
ASSERT_FALSE(exception_state.HadException());
EXPECT_TRUE(sheet->href().IsNull());
EXPECT_EQ(sheet->parentStyleSheet(), nullptr);
......@@ -98,16 +70,15 @@ TEST_F(CSSStyleSheetTest, CreateEmptyCSSStyleSheetWithEmptyCSSStyleSheetInit) {
}
TEST_F(CSSStyleSheetTest,
CreateEmptyCSSStyleSheetWithNonEmptyCSSStyleSheetInit) {
CSSStyleSheetConstructionWithNonEmptyCSSStyleSheetInit) {
DummyExceptionStateForTesting exception_state;
CSSStyleSheetInit init;
init.setMedia(MediaListOrString::FromString("screen, print"));
init.setTitle("test");
init.setAlternate(true);
init.setDisabled(true);
V8TestingScope scope;
DummyExceptionStateForTesting exception_state;
CSSStyleSheet* sheet = GetDocument().createEmptyCSSStyleSheet(
scope.GetScriptState(), init, exception_state);
CSSStyleSheet* sheet =
CSSStyleSheet::Create(GetDocument(), init, exception_state);
ASSERT_FALSE(exception_state.HadException());
EXPECT_TRUE(sheet->href().IsNull());
EXPECT_EQ(sheet->parentStyleSheet(), nullptr);
......@@ -118,6 +89,7 @@ TEST_F(CSSStyleSheetTest,
EXPECT_EQ(sheet->title(), init.title());
EXPECT_TRUE(sheet->AlternateFromConstructor());
EXPECT_TRUE(sheet->disabled());
EXPECT_EQ(sheet->cssRules(exception_state)->length(), 0U);
ASSERT_FALSE(exception_state.HadException());
}
......
......@@ -435,11 +435,9 @@ TEST_F(CustomElementRegistryTest, lookupCustomElementDefinition) {
TEST_F(CustomElementRegistryTest, defineCustomElementWithStyle) {
RuntimeEnabledFeatures::SetConstructableStylesheetsEnabled(true);
V8TestingScope scope;
NonThrowableExceptionState should_not_throw;
ElementDefinitionOptions options;
CSSStyleSheet* sheet = GetDocument().createEmptyCSSStyleSheet(
scope.GetScriptState(), CSSStyleSheetInit(), should_not_throw);
CSSStyleSheet* sheet = CSSStyleSheet::Create(GetDocument(), should_not_throw);
options.setStyle(sheet);
TestCustomElementDefinitionBuilder builder(sheet);
CustomElementDefinition* definition_a =
......
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