75635 2012-01-05 11:19:43 -0800 Add a compile-time assertion for the size of CSSValue 2012-01-05 16:22:18 -0800 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Windows 7 RESOLVED FIXED P2 Normal --- 1 tony rniwa kling koivisto macpherson rniwa oldest_to_newest 530457 0 tony 2012-01-05 11:19:43 -0800 From CSSValue.h: protected: // The bits in this section are only used by specific subclasses but kept here // to maximize struct packing. // CSSPrimitiveValue bits: unsigned char m_primitiveUnitType : 7; // CSSPrimitiveValue::UnitTypes mutable bool m_hasCachedCSSText : 1; bool m_isQuirkValue : 1; // CSSInitialValue bits: bool m_isImplicitInitialValue : 1; // CSSValueList bits: bool m_isSpaceSeparatedValueList : 1; private: unsigned char m_classType : ClassTypeBits; // ClassType ClassTypeBits is 5 bits. Swapping between bools and unsigned chars is bad for MSVC. 530651 1 rniwa 2012-01-05 14:50:40 -0800 It turned out that Visual Studio can pack them nicely as is because the only other POD type used here is unsigned char, which has the same size as bool inside bitfields on Visual Studio. 530677 2 rniwa 2012-01-05 15:14:17 -0800 It appears that http://trac.webkit.org/changeset/101119/trunk/Source/WebCore/css/CSSValue.cpp added a compile-time assertion to make sure CSSValue is less than 4 bytes but this isn't tight enough on some platforms where it can fit in 2 bytes. 530696 3 macpherson 2012-01-05 15:27:48 -0800 Don't forget that CSSValue extends RefCounted<CSSValue>, so best case is 6 bytes for CSSValue, and on some platforms it is 8 bytes due to alignment. 530727 4 121355 rniwa 2012-01-05 15:50:37 -0800 Created attachment 121355 Tightens the assertion 530733 5 121355 tony 2012-01-05 15:55:33 -0800 Comment on attachment 121355 Tightens the assertion View in context: https://bugs.webkit.org/attachment.cgi?id=121355&action=review > Source/WebCore/WebCore.xcodeproj/project.pbxproj:26946 > - 07A6D1EB1491137700051D0C /* MediaFragmentURIParser.cpp in Sources */, > + 07A6D1EB1491137700051D0C /* MediaFragmentURIParser.cpp in Sources */, Not sure if you meant to include this in the change, but it seems harmless (I would probably commit it in a separate change w/o review). 530751 6 rniwa 2012-01-05 16:22:07 -0800 Landed in http://trac.webkit.org/changeset/104236. 121355 2012-01-05 15:50:37 -0800 2012-01-05 16:22:18 -0800 Tightens the assertion bug-75635-20120105155036.patch text/plain 2729 rniwa SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEwNDIzMikKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBACisyMDEyLTAxLTA1ICBSeW9zdWtl IE5pd2EgIDxybml3YUB3ZWJraXQub3JnPgorCisgICAgICAgIEFkZCBhIGNvbXBpbGUtdGltZSBh c3NlcnRpb24gZm9yIHRoZSBzaXplIG9mIENTU1ZhbHVlCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03NTYzNQorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFRpZ2h0ZW5lZCB0aGUgY29tcGlsZS10aW1lIGFzc2Vy dGlvbi4KKworICAgICAgICAqIFdlYkNvcmUueGNvZGVwcm9qL3Byb2plY3QucGJ4cHJvajoKKyAg ICAgICAgKiBjc3MvQ1NTVmFsdWUuY3BwOgorCiAyMDEyLTAxLTA1ICBBZGFtIEJhcnRoICA8YWJh cnRoQHdlYmtpdC5vcmc+CiAKICAgICAgICAgSW50cm9kdWNlIFBsYXRmb3JtIG5hbWVzcGFjZSBm b3IgV2ViQ29yZS9wbGF0Zm9ybQpJbmRleDogU291cmNlL1dlYkNvcmUvV2ViQ29yZS54Y29kZXBy b2ovcHJvamVjdC5wYnhwcm9qCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL1dlYkNvcmUu eGNvZGVwcm9qL3Byb2plY3QucGJ4cHJvagkocmV2aXNpb24gMTA0MjA5KQorKysgU291cmNlL1dl YkNvcmUvV2ViQ29yZS54Y29kZXByb2ovcHJvamVjdC5wYnhwcm9qCSh3b3JraW5nIGNvcHkpCkBA IC0yMzgzMiw3ICsyMzgzMiw3IEBACiAJCQkJRkQ2RjI1MkQxM0Y1RUYwRTAwNjUxNjVGIC8qIE1l ZGlhRWxlbWVudEF1ZGlvU291cmNlTm9kZS5oIGluIEhlYWRlcnMgKi8sCiAJCQkJRTQ0NjEzQUQw Q0Q2MzMxMDAwRkFEQTc1IC8qIE1lZGlhRXJyb3IuaCBpbiBIZWFkZXJzICovLAogCQkJCTRFMTk1 OTIyMEEzOURBQkEwMDIyMEZFNSAvKiBNZWRpYUZlYXR1cmVOYW1lcy5oIGluIEhlYWRlcnMgKi8s Ci0gCQkJCTA3QTZEMUVDMTQ5MTEzNzcwMDA1MUQwQyAvKiBNZWRpYUZyYWdtZW50VVJJUGFyc2Vy LmggaW4gSGVhZGVycyAqLywKKwkJCQkwN0E2RDFFQzE0OTExMzc3MDAwNTFEMEMgLyogTWVkaWFG cmFnbWVudFVSSVBhcnNlci5oIGluIEhlYWRlcnMgKi8sCiAJCQkJQThFQTgwMEUwQTE5NTE2RTAw QThFRjVGIC8qIE1lZGlhTGlzdC5oIGluIEhlYWRlcnMgKi8sCiAJCQkJRTQ0NjEzRTQwQ0Q2ODFB MjAwRkFEQTc1IC8qIE1lZGlhUGxheWVyLmggaW4gSGVhZGVycyAqLywKIAkJCQkwNzZGMEQwRTEy QjgxOTI3MDBDMjZBQTQgLyogTWVkaWFQbGF5ZXJQcml2YXRlQVZGb3VuZGF0aW9uLmggaW4gSGVh ZGVycyAqLywKQEAgLTI2OTQzLDcgKzI2OTQzLDcgQEAKIAkJCQk5NzIwNUFCNzEyMzkyOTEwMDBC MTczODAgLyogTWVkaWFEb2N1bWVudC5jcHAgaW4gU291cmNlcyAqLywKIAkJCQlGRDZGMjUyQzEz RjVFRjBFMDA2NTE2NUYgLyogTWVkaWFFbGVtZW50QXVkaW9Tb3VyY2VOb2RlLmNwcCBpbiBTb3Vy Y2VzICovLAogCQkJCTRFMTk1OTIxMEEzOURBQkEwMDIyMEZFNSAvKiBNZWRpYUZlYXR1cmVOYW1l cy5jcHAgaW4gU291cmNlcyAqLywKLSAJCQkJMDdBNkQxRUIxNDkxMTM3NzAwMDUxRDBDIC8qIE1l ZGlhRnJhZ21lbnRVUklQYXJzZXIuY3BwIGluIFNvdXJjZXMgKi8sCisJCQkJMDdBNkQxRUIxNDkx MTM3NzAwMDUxRDBDIC8qIE1lZGlhRnJhZ21lbnRVUklQYXJzZXIuY3BwIGluIFNvdXJjZXMgKi8s CiAJCQkJQThFQTgwMDkwQTE5NTE2RTAwQThFRjVGIC8qIE1lZGlhTGlzdC5jcHAgaW4gU291cmNl cyAqLywKIAkJCQlFNDQ2MTNFMzBDRDY4MTlGMDBGQURBNzUgLyogTWVkaWFQbGF5ZXIuY3BwIGlu IFNvdXJjZXMgKi8sCiAJCQkJMDcyQzhCMTExMzFDNTE4NjAwQTRGQ0U5IC8qIE1lZGlhUGxheWVy UHJpdmF0ZUFWRm91bmRhdGlvbi5jcHAgaW4gU291cmNlcyAqLywKSW5kZXg6IFNvdXJjZS9XZWJD b3JlL2Nzcy9DU1NWYWx1ZS5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvY3NzL0NT U1ZhbHVlLmNwcAkocmV2aXNpb24gMTA0MjA5KQorKysgU291cmNlL1dlYkNvcmUvY3NzL0NTU1Zh bHVlLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNTksOSArNTksMTEgQEAKIAogbmFtZXNwYWNlIFdl YkNvcmUgewogCi0jaWZkZWYgTkRFQlVHCi1DT01QSUxFX0FTU0VSVCgoc2l6ZW9mKENTU1ZhbHVl KSAtIHNpemVvZihSZWZDb3VudGVkPENTU1ZhbHVlPikpIDw9IDQsIENTU192YWx1ZV9wYWNrc19p bnRvX2ZvdXJfYnl0ZXMpOwotI2VuZGlmCitjbGFzcyBTYW1lU2l6ZUFzQ1NTVmFsdWUgOiBwdWJs aWMgUmVmQ291bnRlZDxTYW1lU2l6ZUFzQ1NTVmFsdWU+IHsKKyAgICB1bnNpZ25lZCBjaGFyIGJp dGZpZWxkc1syXTsKK307CisKK0NPTVBJTEVfQVNTRVJUKHNpemVvZihDU1NWYWx1ZSkgPT0gc2l6 ZW9mKFNhbWVTaXplQXNDU1NWYWx1ZSksIENTU192YWx1ZV9zaG91bGRfc3RheV9zbWFsbCk7CiAK IENTU1ZhbHVlOjpUeXBlIENTU1ZhbHVlOjpjc3NWYWx1ZVR5cGUoKSBjb25zdAogewo=