Skip to content

Commit c831ce8

Browse files
author
Keishi Hattori
committed
Move MediaQuery classes off BlinkGC heap
This CL moves the following classes off the BlinkGC heap MediaQueryExp MediaQuery MediaQuerySet MediaQueryResult in an effort to avoid the crasher crbug.com/699269. BUG=699269 Review-Url: https://codereview.chromium.org/2837023005 Cr-Commit-Position: refs/heads/master@{#467331} (cherry picked from commit d434dda Review-Url: https://codereview.chromium.org/2873433003 . Cr-Commit-Position: refs/branch-heads/3029@{#828} Cr-Branched-From: 939b32e-refs/heads/master@{#454471}
1 parent 72883d8 commit c831ce8

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+209
-216
lines changed

third_party/WebKit/Source/core/css/CSSMediaRule.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ CSSMediaRule::CSSMediaRule(StyleRuleMedia* mediaRule, CSSStyleSheet* parent)
3232

3333
CSSMediaRule::~CSSMediaRule() {}
3434

35-
MediaQuerySet* CSSMediaRule::mediaQueries() const {
35+
RefPtr<MediaQuerySet> CSSMediaRule::mediaQueries() const {
3636
return toStyleRuleMedia(m_groupRule.get())->mediaQueries();
3737
}
3838

third_party/WebKit/Source/core/css/CSSMediaRule.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class CSSMediaRule final : public CSSConditionRule {
5353

5454
CSSRule::Type type() const override { return kMediaRule; }
5555

56-
MediaQuerySet* mediaQueries() const;
56+
RefPtr<MediaQuerySet> mediaQueries() const;
5757

5858
mutable Member<MediaList> m_mediaCSSOMWrapper;
5959
};

third_party/WebKit/Source/core/css/CSSStyleSheet.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,8 @@ void CSSStyleSheet::setDisabled(bool disabled) {
202202
didMutate();
203203
}
204204

205-
void CSSStyleSheet::setMediaQueries(MediaQuerySet* mediaQueries) {
206-
m_mediaQueries = mediaQueries;
205+
void CSSStyleSheet::setMediaQueries(RefPtr<MediaQuerySet> mediaQueries) {
206+
m_mediaQueries = std::move(mediaQueries);
207207
if (m_mediaCSSOMWrapper && m_mediaQueries)
208208
m_mediaCSSOMWrapper->reattach(m_mediaQueries.get());
209209
}
@@ -214,7 +214,7 @@ bool CSSStyleSheet::matchesMediaQueries(const MediaQueryEvaluator& evaluator) {
214214

215215
if (!m_mediaQueries)
216216
return true;
217-
return evaluator.eval(m_mediaQueries, &m_viewportDependentMediaQueryResults,
217+
return evaluator.eval(*m_mediaQueries, &m_viewportDependentMediaQueryResults,
218218
&m_deviceDependentMediaQueryResults);
219219
}
220220

@@ -444,9 +444,6 @@ void CSSStyleSheet::setText(const String& text) {
444444

445445
DEFINE_TRACE(CSSStyleSheet) {
446446
visitor->trace(m_contents);
447-
visitor->trace(m_mediaQueries);
448-
visitor->trace(m_viewportDependentMediaQueryResults);
449-
visitor->trace(m_deviceDependentMediaQueryResults);
450447
visitor->trace(m_ownerNode);
451448
visitor->trace(m_ownerRule);
452449
visitor->trace(m_mediaCSSOMWrapper);

third_party/WebKit/Source/core/css/CSSStyleSheet.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ class CORE_EXPORT CSSStyleSheet final : public StyleSheet {
100100

101101
void clearOwnerRule() { m_ownerRule = nullptr; }
102102
Document* ownerDocument() const;
103-
const MediaQuerySet* mediaQueries() const { return m_mediaQueries; }
104-
void setMediaQueries(MediaQuerySet*);
103+
const MediaQuerySet* mediaQueries() const { return m_mediaQueries.get(); }
104+
void setMediaQueries(RefPtr<MediaQuerySet>);
105105
bool matchesMediaQueries(const MediaQueryEvaluator&);
106106
const MediaQueryResultList& viewportDependentMediaQueryResults() const {
107107
return m_viewportDependentMediaQueryResults;
@@ -164,7 +164,7 @@ class CORE_EXPORT CSSStyleSheet final : public StyleSheet {
164164
bool m_isDisabled = false;
165165
bool m_loadCompleted = false;
166166
String m_title;
167-
Member<MediaQuerySet> m_mediaQueries;
167+
RefPtr<MediaQuerySet> m_mediaQueries;
168168
MediaQueryResultList m_viewportDependentMediaQueryResults;
169169
MediaQueryResultList m_deviceDependentMediaQueryResults;
170170

third_party/WebKit/Source/core/css/MediaList.cpp

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
#include "bindings/core/v8/ExceptionState.h"
2323
#include "core/css/CSSStyleSheet.h"
24-
#include "core/css/MediaQuery.h"
2524
#include "core/css/MediaQueryExp.h"
2625
#include "core/css/parser/MediaQueryParser.h"
2726
#include "wtf/text/StringBuilder.h"
@@ -58,15 +57,15 @@ MediaQuerySet::MediaQuerySet(const MediaQuerySet& o)
5857
m_queries[i] = o.m_queries[i]->copy();
5958
}
6059

61-
MediaQuerySet* MediaQuerySet::create(const String& mediaString) {
60+
RefPtr<MediaQuerySet> MediaQuerySet::create(const String& mediaString) {
6261
if (mediaString.isEmpty())
6362
return MediaQuerySet::create();
6463

6564
return MediaQueryParser::parseMediaQuerySet(mediaString);
6665
}
6766

6867
bool MediaQuerySet::set(const String& mediaString) {
69-
MediaQuerySet* result = create(mediaString);
68+
RefPtr<MediaQuerySet> result = create(mediaString);
7069
m_queries.swap(result->m_queries);
7170
return true;
7271
}
@@ -75,46 +74,46 @@ bool MediaQuerySet::add(const String& queryString) {
7574
// To "parse a media query" for a given string means to follow "the parse
7675
// a media query list" steps and return "null" if more than one media query
7776
// is returned, or else the returned media query.
78-
MediaQuerySet* result = create(queryString);
77+
RefPtr<MediaQuerySet> result = create(queryString);
7978

8079
// Only continue if exactly one media query is found, as described above.
8180
if (result->m_queries.size() != 1)
8281
return true;
8382

84-
MediaQuery* newQuery = result->m_queries[0].release();
83+
std::unique_ptr<MediaQuery> newQuery = std::move(result->m_queries[0]);
8584
ASSERT(newQuery);
8685

8786
// If comparing with any of the media queries in the collection of media
8887
// queries returns true terminate these steps.
8988
for (size_t i = 0; i < m_queries.size(); ++i) {
90-
MediaQuery* query = m_queries[i].get();
91-
if (*query == *newQuery)
89+
MediaQuery& query = *m_queries[i];
90+
if (query == *newQuery)
9291
return true;
9392
}
9493

95-
m_queries.push_back(newQuery);
94+
m_queries.push_back(std::move(newQuery));
9695
return true;
9796
}
9897

9998
bool MediaQuerySet::remove(const String& queryStringToRemove) {
10099
// To "parse a media query" for a given string means to follow "the parse
101100
// a media query list" steps and return "null" if more than one media query
102101
// is returned, or else the returned media query.
103-
MediaQuerySet* result = create(queryStringToRemove);
102+
RefPtr<MediaQuerySet> result = create(queryStringToRemove);
104103

105104
// Only continue if exactly one media query is found, as described above.
106105
if (result->m_queries.size() != 1)
107106
return true;
108107

109-
MediaQuery* newQuery = result->m_queries[0].release();
108+
std::unique_ptr<MediaQuery> newQuery = std::move(result->m_queries[0]);
110109
ASSERT(newQuery);
111110

112111
// Remove any media query from the collection of media queries for which
113112
// comparing with the media query returns true.
114113
bool found = false;
115114
for (size_t i = 0; i < m_queries.size(); ++i) {
116-
MediaQuery* query = m_queries[i].get();
117-
if (*query == *newQuery) {
115+
MediaQuery& query = *m_queries[i];
116+
if (query == *newQuery) {
118117
m_queries.remove(i);
119118
--i;
120119
found = true;
@@ -124,8 +123,8 @@ bool MediaQuerySet::remove(const String& queryStringToRemove) {
124123
return found;
125124
}
126125

127-
void MediaQuerySet::addMediaQuery(MediaQuery* mediaQuery) {
128-
m_queries.push_back(mediaQuery);
126+
void MediaQuerySet::addMediaQuery(std::unique_ptr<MediaQuery> mediaQuery) {
127+
m_queries.push_back(std::move(mediaQuery));
129128
}
130129

131130
String MediaQuerySet::mediaText() const {
@@ -142,16 +141,13 @@ String MediaQuerySet::mediaText() const {
142141
return text.toString();
143142
}
144143

145-
DEFINE_TRACE(MediaQuerySet) {
146-
visitor->trace(m_queries);
147-
}
148-
149-
MediaList::MediaList(MediaQuerySet* mediaQueries, CSSStyleSheet* parentSheet)
144+
MediaList::MediaList(RefPtr<MediaQuerySet> mediaQueries,
145+
CSSStyleSheet* parentSheet)
150146
: m_mediaQueries(mediaQueries),
151147
m_parentStyleSheet(parentSheet),
152148
m_parentRule(nullptr) {}
153149

154-
MediaList::MediaList(MediaQuerySet* mediaQueries, CSSRule* parentRule)
150+
MediaList::MediaList(RefPtr<MediaQuerySet> mediaQueries, CSSRule* parentRule)
155151
: m_mediaQueries(mediaQueries),
156152
m_parentStyleSheet(nullptr),
157153
m_parentRule(parentRule) {}
@@ -166,7 +162,8 @@ void MediaList::setMediaText(const String& value) {
166162
}
167163

168164
String MediaList::item(unsigned index) const {
169-
const HeapVector<Member<MediaQuery>>& queries = m_mediaQueries->queryVector();
165+
const Vector<std::unique_ptr<MediaQuery>>& queries =
166+
m_mediaQueries->queryVector();
170167
if (index < queries.size())
171168
return queries[index]->cssText();
172169
return String();
@@ -202,13 +199,12 @@ void MediaList::appendMedium(const String& medium,
202199
m_parentStyleSheet->didMutate();
203200
}
204201

205-
void MediaList::reattach(MediaQuerySet* mediaQueries) {
202+
void MediaList::reattach(RefPtr<MediaQuerySet> mediaQueries) {
206203
ASSERT(mediaQueries);
207204
m_mediaQueries = mediaQueries;
208205
}
209206

210207
DEFINE_TRACE(MediaList) {
211-
visitor->trace(m_mediaQueries);
212208
visitor->trace(m_parentStyleSheet);
213209
visitor->trace(m_parentRule);
214210
}

third_party/WebKit/Source/core/css/MediaList.h

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
#include "bindings/core/v8/ScriptWrappable.h"
2626
#include "core/CoreExport.h"
27+
#include "core/css/MediaQuery.h"
2728
#include "core/dom/ExceptionCode.h"
2829
#include "platform/heap/Handle.h"
2930
#include "wtf/Forward.h"
@@ -38,46 +39,51 @@ class ExceptionState;
3839
class MediaList;
3940
class MediaQuery;
4041

41-
class CORE_EXPORT MediaQuerySet : public GarbageCollected<MediaQuerySet> {
42+
class CORE_EXPORT MediaQuerySet : public RefCounted<MediaQuerySet> {
4243
public:
43-
static MediaQuerySet* create() { return new MediaQuerySet(); }
44-
static MediaQuerySet* create(const String& mediaString);
44+
static RefPtr<MediaQuerySet> create() {
45+
return adoptRef(new MediaQuerySet());
46+
}
47+
static RefPtr<MediaQuerySet> create(const String& mediaString);
4548

4649
bool set(const String&);
4750
bool add(const String&);
4851
bool remove(const String&);
4952

50-
void addMediaQuery(MediaQuery*);
53+
void addMediaQuery(std::unique_ptr<MediaQuery>);
5154

52-
const HeapVector<Member<MediaQuery>>& queryVector() const {
55+
const Vector<std::unique_ptr<MediaQuery>>& queryVector() const {
5356
return m_queries;
5457
}
5558

5659
String mediaText() const;
5760

58-
MediaQuerySet* copy() const { return new MediaQuerySet(*this); }
61+
RefPtr<MediaQuerySet> copy() const {
62+
return adoptRef(new MediaQuerySet(*this));
63+
}
5964

6065
DECLARE_TRACE();
6166

6267
private:
6368
MediaQuerySet();
6469
MediaQuerySet(const MediaQuerySet&);
6570

66-
HeapVector<Member<MediaQuery>> m_queries;
71+
Vector<std::unique_ptr<MediaQuery>> m_queries;
6772
};
6873

69-
class MediaList final : public GarbageCollected<MediaList>,
74+
class MediaList final : public GarbageCollectedFinalized<MediaList>,
7075
public ScriptWrappable {
7176
DEFINE_WRAPPERTYPEINFO();
7277

7378
public:
74-
static MediaList* create(MediaQuerySet* mediaQueries,
79+
static MediaList* create(RefPtr<MediaQuerySet> mediaQueries,
7580
CSSStyleSheet* parentSheet) {
76-
return new MediaList(mediaQueries, parentSheet);
81+
return new MediaList(std::move(mediaQueries), parentSheet);
7782
}
7883

79-
static MediaList* create(MediaQuerySet* mediaQueries, CSSRule* parentRule) {
80-
return new MediaList(mediaQueries, parentRule);
84+
static MediaList* create(RefPtr<MediaQuerySet> mediaQueries,
85+
CSSRule* parentRule) {
86+
return new MediaList(std::move(mediaQueries), parentRule);
8187
}
8288

8389
unsigned length() const { return m_mediaQueries->queryVector().size(); }
@@ -94,15 +100,15 @@ class MediaList final : public GarbageCollected<MediaList>,
94100

95101
const MediaQuerySet* queries() const { return m_mediaQueries.get(); }
96102

97-
void reattach(MediaQuerySet*);
103+
void reattach(RefPtr<MediaQuerySet>);
98104

99105
DECLARE_TRACE();
100106

101107
private:
102-
MediaList(MediaQuerySet*, CSSStyleSheet* parentSheet);
103-
MediaList(MediaQuerySet*, CSSRule* parentRule);
108+
MediaList(RefPtr<MediaQuerySet>, CSSStyleSheet* parentSheet);
109+
MediaList(RefPtr<MediaQuerySet>, CSSRule* parentRule);
104110

105-
Member<MediaQuerySet> m_mediaQueries;
111+
RefPtr<MediaQuerySet> m_mediaQueries;
106112
// Cleared in ~CSSStyleSheet destructor when oilpan is not enabled.
107113
Member<CSSStyleSheet> m_parentStyleSheet;
108114
// Cleared in the ~CSSMediaRule and ~CSSImportRule destructors when oilpan is

third_party/WebKit/Source/core/css/MediaQuery.cpp

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -61,29 +61,29 @@ String MediaQuery::serialize() const {
6161
result.append(" and ");
6262
}
6363

64-
result.append(m_expressions.at(0)->serialize());
64+
result.append(m_expressions.at(0).serialize());
6565
for (size_t i = 1; i < m_expressions.size(); ++i) {
6666
result.append(" and ");
67-
result.append(m_expressions.at(i)->serialize());
67+
result.append(m_expressions.at(i).serialize());
6868
}
6969
return result.toString();
7070
}
7171

72-
static bool expressionCompare(const Member<MediaQueryExp>& a,
73-
const Member<MediaQueryExp>& b) {
74-
return codePointCompare(a->serialize(), b->serialize()) < 0;
72+
static bool expressionCompare(const MediaQueryExp& a, const MediaQueryExp& b) {
73+
return codePointCompare(a.serialize(), b.serialize()) < 0;
7574
}
7675

77-
MediaQuery* MediaQuery::createNotAll() {
78-
return new MediaQuery(MediaQuery::Not, MediaTypeNames::all,
79-
ExpressionHeapVector());
76+
std::unique_ptr<MediaQuery> MediaQuery::createNotAll() {
77+
return WTF::makeUnique<MediaQuery>(MediaQuery::Not, MediaTypeNames::all,
78+
ExpressionHeapVector());
8079
}
8180

82-
MediaQuery* MediaQuery::create(RestrictorType restrictor,
83-
String mediaType,
84-
ExpressionHeapVector expressions) {
85-
return new MediaQuery(restrictor, std::move(mediaType),
86-
std::move(expressions));
81+
std::unique_ptr<MediaQuery> MediaQuery::create(
82+
RestrictorType restrictor,
83+
String mediaType,
84+
ExpressionHeapVector expressions) {
85+
return WTF::makeUnique<MediaQuery>(restrictor, std::move(mediaType),
86+
std::move(expressions));
8787
}
8888

8989
MediaQuery::MediaQuery(RestrictorType restrictor,
@@ -95,11 +95,11 @@ MediaQuery::MediaQuery(RestrictorType restrictor,
9595
nonCopyingSort(m_expressions.begin(), m_expressions.end(), expressionCompare);
9696

9797
// Remove all duplicated expressions.
98-
MediaQueryExp* key = 0;
98+
MediaQueryExp key = MediaQueryExp::invalid();
9999
for (int i = m_expressions.size() - 1; i >= 0; --i) {
100-
MediaQueryExp* exp = m_expressions.at(i).get();
100+
MediaQueryExp exp = m_expressions.at(i);
101101

102-
if (key && *exp == *key)
102+
if (exp == key)
103103
m_expressions.remove(i);
104104
else
105105
key = exp;
@@ -112,7 +112,7 @@ MediaQuery::MediaQuery(const MediaQuery& o)
112112
m_serializationCache(o.m_serializationCache) {
113113
m_expressions.reserveInitialCapacity(o.m_expressions.size());
114114
for (unsigned i = 0; i < o.m_expressions.size(); ++i)
115-
m_expressions.push_back(o.m_expressions[i]->copy());
115+
m_expressions.push_back(o.m_expressions[i]);
116116
}
117117

118118
MediaQuery::~MediaQuery() {}
@@ -130,8 +130,4 @@ String MediaQuery::cssText() const {
130130
return m_serializationCache;
131131
}
132132

133-
DEFINE_TRACE(MediaQuery) {
134-
visitor->trace(m_expressions);
135-
}
136-
137133
} // namespace blink

0 commit comments

Comments
 (0)