Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(33)

Unified Diff: third_party/WebKit/Source/core/html/HTMLTableElement.cpp

Issue 2680843006: Tidy DEFINE_(THREAD_SAFE_)STATIC_LOCAL() implementations. (Closed)
Patch Set: explain safety of HTMLTableSectionElement singletons Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/html/HTMLTableElement.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLTableElement.cpp b/third_party/WebKit/Source/core/html/HTMLTableElement.cpp
index 6d507ba3f0712a285d36db17bfa20c961dd39415..33162549edbe422699d15c50139458fc469bc8cc 100644
--- a/third_party/WebKit/Source/core/html/HTMLTableElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLTableElement.cpp
@@ -453,8 +453,20 @@ HTMLTableElement::additionalPresentationAttributeStyle() {
// Setting the border to 'hidden' allows it to win over any border
// set on the table's cells during border-conflict resolution.
if (m_rulesAttr != UnsetRules) {
+ // Note: this (Mutable)StylePropertySet singleton has an embedded
+ // CSSStyleDeclaration reference (m_cssomWrapper), which is a
+ // ScriptWrappable (=> it can have a v8 wrapper object). Exposing
+ // and sharing those via static locals is unsafe, as the singleton
+ // really is accessible across window contexts within a renderer
+ // process.
+ //
+ // However, that CSSStyleDeclaration is never instantiated for these
+ // presentation attribute style properties, hence this singleton (and
+ // the ones below) are considered safe. Indicate that by disabling
+ // the no-ScriptWrappables-in-singletons verification check.
haraken 2017/02/11 10:25:20 Your analysis looks correct, but it looks better t
sof 2017/02/11 12:09:11 That could probably be done without it being too a
DEFINE_STATIC_LOCAL(StylePropertySet, solidBorderStyle,
- (createBorderStyle(CSSValueHidden)));
+ (createBorderStyle(CSSValueHidden)),
+ CheckScriptWrappable::No);
return &solidBorderStyle;
}
return nullptr;
@@ -462,11 +474,13 @@ HTMLTableElement::additionalPresentationAttributeStyle() {
if (m_borderColorAttr) {
DEFINE_STATIC_LOCAL(StylePropertySet, solidBorderStyle,
- (createBorderStyle(CSSValueSolid)));
+ (createBorderStyle(CSSValueSolid)),
+ CheckScriptWrappable::No);
return &solidBorderStyle;
}
DEFINE_STATIC_LOCAL(StylePropertySet, outsetBorderStyle,
- (createBorderStyle(CSSValueOutset)));
+ (createBorderStyle(CSSValueOutset)),
+ CheckScriptWrappable::No);
return &outsetBorderStyle;
}
@@ -570,11 +584,13 @@ const StylePropertySet* HTMLTableElement::additionalGroupStyle(bool rows) {
if (rows) {
DEFINE_STATIC_LOCAL(StylePropertySet, rowBorderStyle,
- (createGroupBorderStyle(true)));
+ (createGroupBorderStyle(true)),
+ CheckScriptWrappable::No);
return &rowBorderStyle;
}
DEFINE_STATIC_LOCAL(StylePropertySet, columnBorderStyle,
- (createGroupBorderStyle(false)));
+ (createGroupBorderStyle(false)),
+ CheckScriptWrappable::No);
return &columnBorderStyle;
}

Powered by Google App Engine
This is Rietveld 408576698