Move MediaQuery classes off BlinkGC heap (issue 2837023005 by keishi@chromium.org)

0 просмотров
Перейти к первому непрочитанному сообщению

kei...@chromium.org

не прочитано,
26 апр. 2017 г., 04:52:1426.04.2017
– oilpan-...@chromium.org, chromium...@chromium.org, kenneth.ch...@gmail.com, yo...@yoav.ws, blink-rev...@chromium.org, gavinp+p...@chromium.org, blink-rev...@chromium.org, kozyatins...@chromium.org, blink-re...@chromium.org, sigb...@opera.com, eae+bli...@chromium.org, loading-rev...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, caseq...@chromium.org, apavlo...@chromium.org, alexis...@intel.com, blink-...@chromium.org, kinuko...@chromium.org, devtools...@chromium.org, rob....@samsung.com
Reviewers: oilpan-reviews
CL: https://codereview.chromium.org/2837023005/

Message:
It looks like none of my CHECKs worked. This CL moves MediaQuery related classes
off the BlinkGC heap.

Description:
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

Affected files (+213, -224 lines):
M third_party/WebKit/Source/core/css/ActiveStyleSheetsTest.cpp
M third_party/WebKit/Source/core/css/CSSMediaRule.h
M third_party/WebKit/Source/core/css/CSSMediaRule.cpp
M third_party/WebKit/Source/core/css/CSSStyleSheet.h
M third_party/WebKit/Source/core/css/CSSStyleSheet.cpp
M third_party/WebKit/Source/core/css/MediaList.h
M third_party/WebKit/Source/core/css/MediaList.cpp
M third_party/WebKit/Source/core/css/MediaQuery.h
M third_party/WebKit/Source/core/css/MediaQuery.cpp
M third_party/WebKit/Source/core/css/MediaQueryEvaluator.h
M third_party/WebKit/Source/core/css/MediaQueryEvaluator.cpp
M third_party/WebKit/Source/core/css/MediaQueryEvaluatorTest.cpp
M third_party/WebKit/Source/core/css/MediaQueryExp.h
M third_party/WebKit/Source/core/css/MediaQueryExp.cpp
M third_party/WebKit/Source/core/css/MediaQueryList.h
M third_party/WebKit/Source/core/css/MediaQueryList.cpp
M third_party/WebKit/Source/core/css/MediaQueryMatcher.cpp
M third_party/WebKit/Source/core/css/MediaQueryMatcherTest.cpp
M third_party/WebKit/Source/core/css/MediaQuerySetTest.cpp
M third_party/WebKit/Source/core/css/RuleFeature.cpp
M third_party/WebKit/Source/core/css/RuleSet.cpp
M third_party/WebKit/Source/core/css/StyleMedia.cpp
M third_party/WebKit/Source/core/css/StyleRule.h
M third_party/WebKit/Source/core/css/StyleRule.cpp
M third_party/WebKit/Source/core/css/StyleRuleImport.h
M third_party/WebKit/Source/core/css/StyleRuleImport.cpp
M third_party/WebKit/Source/core/css/parser/MediaConditionTest.cpp
M third_party/WebKit/Source/core/css/parser/MediaQueryParser.h
M third_party/WebKit/Source/core/css/parser/MediaQueryParser.cpp
M third_party/WebKit/Source/core/css/parser/SizesAttributeParser.h
M third_party/WebKit/Source/core/css/parser/SizesAttributeParser.cpp
M third_party/WebKit/Source/core/css/resolver/MediaQueryResult.h
M third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.h
M third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.cpp
M third_party/WebKit/Source/core/css/resolver/ViewportStyleResolver.h
M third_party/WebKit/Source/core/css/resolver/ViewportStyleResolver.cpp
M third_party/WebKit/Source/core/dom/StyleElement.cpp
M third_party/WebKit/Source/core/dom/StyleEngine.cpp
M third_party/WebKit/Source/core/html/HTMLSourceElement.cpp
M third_party/WebKit/Source/core/html/LinkStyle.cpp
M third_party/WebKit/Source/core/html/parser/HTMLPreloadScanner.cpp
M third_party/WebKit/Source/core/inspector/InspectorCSSAgent.cpp
M third_party/WebKit/Source/core/loader/LinkLoader.cpp


har...@chromium.org

не прочитано,
26 апр. 2017 г., 05:09:3226.04.2017
– kei...@chromium.org, oilpan-...@chromium.org, chromium...@chromium.org, kenneth.ch...@gmail.com, yo...@yoav.ws, blink-rev...@chromium.org, gavinp+p...@chromium.org, blink-rev...@chromium.org, kozyatins...@chromium.org, blink-re...@chromium.org, sigb...@opera.com, eae+bli...@chromium.org, loading-rev...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, caseq...@chromium.org, apavlo...@chromium.org, alexis...@intel.com, blink-...@chromium.org, kinuko...@chromium.org, devtools...@chromium.org, rob....@samsung.com
The approach looks good.

- Avoid using PassRefPtr since we're deprecating it.
- Add std::move() to places where you're passing ownership.




https://codereview.chromium.org/2837023005/diff/20002/third_party/WebKit/Source/core/css/MediaList.h
File third_party/WebKit/Source/core/css/MediaList.h (right):

https://codereview.chromium.org/2837023005/diff/20002/third_party/WebKit/Source/core/css/MediaList.h#newcode84
third_party/WebKit/Source/core/css/MediaList.h:84: return new
MediaList(media_queries, parent_rule);

std::move(media_queries)?

https://codereview.chromium.org/2837023005/

sigb...@opera.com

не прочитано,
26 апр. 2017 г., 05:16:1226.04.2017
– kei...@chromium.org, oilpan-...@chromium.org, har...@chromium.org, chromium...@chromium.org, kenneth.ch...@gmail.com, yo...@yoav.ws, blink-rev...@chromium.org, gavinp+p...@chromium.org, blink-rev...@chromium.org, kozyatins...@chromium.org, blink-re...@chromium.org, eae+bli...@chromium.org, loading-rev...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, caseq...@chromium.org, apavlo...@chromium.org, alexis...@intel.com, blink-...@chromium.org, kinuko...@chromium.org, devtools...@chromium.org, rob....@samsung.com

kei...@chromium.org

не прочитано,
26 апр. 2017 г., 05:44:3626.04.2017
– oilpan-...@chromium.org, har...@chromium.org, sigb...@opera.com, chromium...@chromium.org, kenneth.ch...@gmail.com, yo...@yoav.ws, blink-rev...@chromium.org, gavinp+p...@chromium.org, blink-rev...@chromium.org, kozyatins...@chromium.org, blink-re...@chromium.org, sigb...@opera.com, eae+bli...@chromium.org, loading-rev...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, caseq...@chromium.org, apavlo...@chromium.org, alexis...@intel.com, blink-...@chromium.org, kinuko...@chromium.org, devtools...@chromium.org, rob....@samsung.com
Removed PassRefPtr



https://codereview.chromium.org/2837023005/diff/20002/third_party/WebKit/Source/core/css/MediaList.h
File third_party/WebKit/Source/core/css/MediaList.h (right):

https://codereview.chromium.org/2837023005/diff/20002/third_party/WebKit/Source/core/css/MediaList.h#newcode84
third_party/WebKit/Source/core/css/MediaList.h:84: return new
MediaList(media_queries, parent_rule);
On 2017/04/26 09:09:32, haraken wrote:
>
> std::move(media_queries)?

Done.


https://codereview.chromium.org/2837023005/diff/20002/third_party/WebKit/Source/core/css/MediaQuery.cpp
File third_party/WebKit/Source/core/css/MediaQuery.cpp (right):

https://codereview.chromium.org/2837023005/diff/20002/third_party/WebKit/Source/core/css/MediaQuery.cpp#newcode102
third_party/WebKit/Source/core/css/MediaQuery.cpp:102: CHECK(String() ==
String());
On 2017/04/26 09:16:11, sof wrote:
> accidental addition?

Oops. Sorry.

https://codereview.chromium.org/2837023005/

har...@chromium.org

не прочитано,
26 апр. 2017 г., 07:15:2526.04.2017
– kei...@chromium.org, oilpan-...@chromium.org, sigb...@opera.com, chromium...@chromium.org, kenneth.ch...@gmail.com, yo...@yoav.ws, blink-rev...@chromium.org, gavinp+p...@chromium.org, blink-rev...@chromium.org, kozyatins...@chromium.org, blink-re...@chromium.org, sigb...@opera.com, eae+bli...@chromium.org, loading-rev...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, caseq...@chromium.org, apavlo...@chromium.org, alexis...@intel.com, blink-...@chromium.org, kinuko...@chromium.org, devtools...@chromium.org, rob....@samsung.com
LGTM



https://codereview.chromium.org/2837023005/diff/50001/third_party/WebKit/Source/core/css/MediaQuery.cpp
File third_party/WebKit/Source/core/css/MediaQuery.cpp (right):

https://codereview.chromium.org/2837023005/diff/50001/third_party/WebKit/Source/core/css/MediaQuery.cpp#newcode77
third_party/WebKit/Source/core/css/MediaQuery.cpp:77: return
WTF::WrapUnique(new MediaQuery(MediaQuery::kNot, MediaTypeNames::all,

MakeUnique.

https://codereview.chromium.org/2837023005/diff/50001/third_party/WebKit/Source/core/css/MediaQuery.cpp#newcode85
third_party/WebKit/Source/core/css/MediaQuery.cpp:85: return
WTF::WrapUnique(new MediaQuery(restrictor, std::move(media_type),

MakeUnique.

https://codereview.chromium.org/2837023005/diff/50001/third_party/WebKit/Source/core/css/MediaQuery.h
File third_party/WebKit/Source/core/css/MediaQuery.h (right):

https://codereview.chromium.org/2837023005/diff/50001/third_party/WebKit/Source/core/css/MediaQuery.h#newcode62
third_party/WebKit/Source/core/css/MediaQuery.h:62: return
WTF::WrapUnique(new MediaQuery(*this));

MakeUnique

https://codereview.chromium.org/2837023005/

kei...@chromium.org

не прочитано,
26 апр. 2017 г., 09:28:5826.04.2017
– oilpan-...@chromium.org, har...@chromium.org, sigb...@opera.com, chromium...@chromium.org, kenneth.ch...@gmail.com, yo...@yoav.ws, blink-rev...@chromium.org, gavinp+p...@chromium.org, blink-rev...@chromium.org, kozyatins...@chromium.org, blink-re...@chromium.org, sigb...@opera.com, eae+bli...@chromium.org, loading-rev...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, caseq...@chromium.org, apavlo...@chromium.org, alexis...@intel.com, blink-...@chromium.org, kinuko...@chromium.org, devtools...@chromium.org, rob....@samsung.com

https://codereview.chromium.org/2837023005/diff/50001/third_party/WebKit/Source/core/css/MediaQuery.cpp
File third_party/WebKit/Source/core/css/MediaQuery.cpp (right):

https://codereview.chromium.org/2837023005/diff/50001/third_party/WebKit/Source/core/css/MediaQuery.cpp#newcode77
third_party/WebKit/Source/core/css/MediaQuery.cpp:77: return
WTF::WrapUnique(new MediaQuery(MediaQuery::kNot, MediaTypeNames::all,
On 2017/04/26 11:15:25, haraken wrote:
>
> MakeUnique.

Done.


https://codereview.chromium.org/2837023005/diff/50001/third_party/WebKit/Source/core/css/MediaQuery.cpp#newcode85
third_party/WebKit/Source/core/css/MediaQuery.cpp:85: return
WTF::WrapUnique(new MediaQuery(restrictor, std::move(media_type),
On 2017/04/26 11:15:25, haraken wrote:
>
> MakeUnique.

Done.


https://codereview.chromium.org/2837023005/diff/50001/third_party/WebKit/Source/core/css/MediaQuery.h
File third_party/WebKit/Source/core/css/MediaQuery.h (right):

https://codereview.chromium.org/2837023005/diff/50001/third_party/WebKit/Source/core/css/MediaQuery.h#newcode62
third_party/WebKit/Source/core/css/MediaQuery.h:62: return
WTF::WrapUnique(new MediaQuery(*this));
On 2017/04/26 11:15:25, haraken wrote:
>
> MakeUnique

Done.

https://codereview.chromium.org/2837023005/

commit-bot@chromium.org via codereview.chromium.org

не прочитано,
26 апр. 2017 г., 09:29:2026.04.2017
– kei...@chromium.org, oilpan-...@chromium.org, har...@chromium.org, sigb...@opera.com, commi...@chromium.org, chromium...@chromium.org, kenneth.ch...@gmail.com, yo...@yoav.ws, blink-rev...@chromium.org, gavinp+p...@chromium.org, blink-rev...@chromium.org, kozyatins...@chromium.org, blink-re...@chromium.org, sigb...@opera.com, eae+bli...@chromium.org, loading-rev...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, caseq...@chromium.org, apavlo...@chromium.org, alexis...@intel.com, blink-...@chromium.org, kinuko...@chromium.org, devtools...@chromium.org, rob....@samsung.com

commit-bot@chromium.org via codereview.chromium.org

не прочитано,
26 апр. 2017 г., 11:46:1326.04.2017
– kei...@chromium.org, oilpan-...@chromium.org, har...@chromium.org, sigb...@opera.com, commi...@chromium.org, chromium...@chromium.org, kenneth.ch...@gmail.com, yo...@yoav.ws, blink-rev...@chromium.org, gavinp+p...@chromium.org, blink-rev...@chromium.org, kozyatins...@chromium.org, blink-re...@chromium.org, sigb...@opera.com, eae+bli...@chromium.org, loading-rev...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, blink-re...@chromium.org, dglazko...@chromium.org, caseq...@chromium.org, apavlo...@chromium.org, alexis...@intel.com, blink-...@chromium.org, kinuko...@chromium.org, devtools...@chromium.org, rob....@samsung.com
Ответить всем
Отправить сообщение автору
Переслать
0 новых сообщений