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

Issue 699043003: bindings: Makes CSSRule inherit from ScriptWrappable. (Closed)

Created:
6 years, 1 month ago by Yuki
Modified:
6 years, 1 month ago
Reviewers:
haraken, Timothy Loh
CC:
blink-reviews, kenneth.christiansen, Yoav Weiss, arv+blink, blink-reviews-css, dglazkov+blink, ed+blinkwatch_opera.com, blink-reviews-bindings_chromium.org, Inactive, darktears, apavlov+blink_chromium.org, rwlbuis
Project:
blink
Visibility:
Public.

Description

bindings: Makes CSSRule inherit from ScriptWrappable. Makes CSSRule and its subclasses inherit from ScriptWrappable. Amongst top 10 mobile web sites, only one web site instantiates only one instance of CSSRule unless Developer Tools is opened. Most of web pages don't instantiate CSSRules and its subclasses (unless Developer Tools is opened), so it should be okay to make CSSRule inherit from ScriptWrappable, although it increases the size of CSSRule by 4 or 8 bytes (= the size of v8::Persistent). BUG=390065 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185166

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -102 lines) Patch
D Source/bindings/core/v8/custom/V8CSSRuleCustom.cpp View 1 chunk +0 lines, -83 lines 0 comments Download
M Source/bindings/core/v8/custom/custom.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M Source/core/css/CSSCharsetRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSFilterRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSFontFaceRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSImportRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSKeyframeRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSKeyframesRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSMediaRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSPageRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSRule.h View 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/css/CSSRule.cpp View 1 chunk +1 line, -13 lines 0 comments Download
M Source/core/css/CSSRule.idl View 2 chunks +1 line, -4 lines 0 comments Download
M Source/core/css/CSSStyleRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSSupportsRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSUnknownRule.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/css/CSSViewportRule.h View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
Yuki
Could you review this CL?
6 years, 1 month ago (2014-11-11 06:59:50 UTC) #2
haraken
LGTM timothy: Are you OK with this change? I guess it's OK to add 8 ...
6 years, 1 month ago (2014-11-11 08:01:20 UTC) #4
Timothy Loh
On 2014/11/11 08:01:20, haraken wrote: > LGTM > > timothy: Are you OK with this ...
6 years, 1 month ago (2014-11-11 17:31:57 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/699043003/1
6 years, 1 month ago (2014-11-12 02:01:07 UTC) #7
commit-bot: I haz the power
6 years, 1 month ago (2014-11-12 02:03:53 UTC) #8
Message was sent while issue was closed.
Committed patchset #1 (id:1) as 185166

Powered by Google App Engine
This is Rietveld 408576698