42447 2010-07-16 04:51:10 -0700 DeviceOrientationEvent.h should not forward-declare DeviceOrientation 2010-07-16 10:19:42 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 0 hans webkit-unassigned commit-queue darin steveblock oldest_to_newest 252459 0 hans 2010-07-16 04:51:10 -0700 Because DeviceOrientationEvent has a member m_orientation of type RefPtr<DeviceOrientation>, the generated ~DeviceOrientationEvent() will destruct the RefPtr and eventually end up doing ptr->deref() in PassRefPtr.h:58, with ptr as an incomplete type. Clang gives an error about this when building Chrome: In file included from third_party/WebKit/WebCore/dom/Document.cpp:27: In file included from third_party/WebKit/WebCore/dom/Document.h:29: In file included from third_party/WebKit/WebCore/loader/CachedResourceHandle.h:29: In file included from third_party/WebKit/WebCore/loader/CachedResource.h:28: In file included from third_party/WebKit/WebCore/platform/text/PlatformString.h:28: In file included from third_party/WebKit/JavaScriptCore/wtf/text/WTFString.h:28: In file included from third_party/WebKit/JavaScriptCore/wtf/text/StringImpl.h:28: In file included from third_party/WebKit/JavaScriptCore/wtf/CrossThreadRefCounted.h:35: third_party/WebKit/JavaScriptCore/wtf/PassRefPtr.h:58:16: error: member access into incomplete type 'WebCore::DeviceOrientation' ptr->deref(); ^ In file included from third_party/WebKit/WebCore/dom/Document.cpp:27: In file included from third_party/WebKit/WebCore/dom/Document.h:29: In file included from third_party/WebKit/WebCore/loader/CachedResourceHandle.h:29: In file included from third_party/WebKit/WebCore/loader/CachedResource.h:28: In file included from third_party/WebKit/WebCore/platform/text/PlatformString.h:28: In file included from third_party/WebKit/JavaScriptCore/wtf/text/WTFString.h:28: In file included from third_party/WebKit/JavaScriptCore/wtf/text/StringImpl.h:32: In file included from third_party/WebKit/JavaScriptCore/wtf/Vector.h:28: In file included from third_party/WebKit/JavaScriptCore/wtf/VectorTraits.h:25: third_party/WebKit/JavaScriptCore/wtf/RefPtr.h:56:35: note: in instantiation of function template specialization 'WTF::derefIfNotNull<WebCore::DeviceOrientation>' requested here ALWAYS_INLINE ~RefPtr() { derefIfNotNull(m_ptr); } ^ In file included from third_party/WebKit/WebCore/dom/Document.cpp:47: third_party/WebKit/WebCore/dom/DeviceOrientationEvent.h:35:7: note: in instantiation of member function 'WTF::RefPtr<WebCore::DeviceOrientation>::~RefPtr' requested here class DeviceOrientationEvent : public Event { ^ third_party/WebKit/WebCore/dom/DeviceOrientationEvent.h:33:7: note: forward declaration of 'WebCore::DeviceOrientation' class DeviceOrientation; 252460 1 61795 hans 2010-07-16 04:59:11 -0700 Created attachment 61795 Patch 252464 2 61795 steveblock 2010-07-16 05:11:16 -0700 Comment on attachment 61795 Patch r=me 252490 3 61795 commit-queue 2010-07-16 06:50:47 -0700 Comment on attachment 61795 Patch Clearing flags on attachment: 61795 Committed r63544: <http://trac.webkit.org/changeset/63544> 252491 4 commit-queue 2010-07-16 06:50:52 -0700 All reviewed patches have been landed. Closing bug. 252560 5 darin 2010-07-16 09:36:55 -0700 Adding the include is one solution in a case like this. Another is to explicitly declare the virtual destructor and define it in the .cpp file. It will have just an empty body, but it will make sure that people who include the header don’t have to compile the destructor. 252566 6 hans 2010-07-16 09:47:48 -0700 (In reply to comment #5) > Adding the include is one solution in a case like this. Another is to explicitly declare the virtual destructor and define it in the .cpp file. It will have just an empty body, but it will make sure that people who include the header don’t have to compile the destructor. Ah, I didn't think of that. Do you think we should change it? 252580 7 hans 2010-07-16 10:19:42 -0700 Uploaded patch to Bug 42466 that implements Darin's suggestion, as it reduces the header size. 61795 2010-07-16 04:59:11 -0700 2010-07-16 06:50:47 -0700 Patch bug-42447-20100716125909.patch text/plain 1241 hans ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg YjgzYzcwZDE5MjVjOWVmOWU1NjZlY2FlOGU3YzM5MWNlZDFiZDE2ZS4uZmQxNmQ1YzdiODBiNzM2 ZmViODU1YzI1YzNjZTgwMTU0ZDA0YzE5MCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAxMC0wNy0xNiAgSGFu cyBXZW5uYm9yZyAgPGhhbnNAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIERldmljZU9yaWVudGF0aW9uRXZlbnQuaCBzaG91bGQg bm90IGZvcndhcmQtZGVjbGFyZSBEZXZpY2VPcmllbnRhdGlvbgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDI0NDcKKworICAgICAgICBXaGVuIGRlc3Ry dWN0aW5nIG1fb3JpZW50YXRpb24sIERldmljZU9yaWVudGF0aW9uIGNhbm5vdCBiZSBhbiBpbmNv bXBsZXRlIHR5cGUuCisKKyAgICAgICAgKiBkb20vRGV2aWNlT3JpZW50YXRpb25FdmVudC5oOgor CiAyMDEwLTA3LTE2ICBOaWtvbGFzIFppbW1lcm1hbm4gIDxuemltbWVybWFubkByaW0uY29tPgog CiAgICAgICAgIFJldmlld2VkIGJ5IERpcmsgU2NodWx6ZS4KZGlmZiAtLWdpdCBhL1dlYkNvcmUv ZG9tL0RldmljZU9yaWVudGF0aW9uRXZlbnQuaCBiL1dlYkNvcmUvZG9tL0RldmljZU9yaWVudGF0 aW9uRXZlbnQuaAppbmRleCA2MDRiMzVmNDEzMzM3ZTNiZWQwOThhODQ1MTMzZGQ2Yjg1ZjZhM2Ni Li4xNjViMWRmMjc2NzhmMzlhYzg2ODg3NWI5ZGMyYzY0NTQ4ODU1YWViIDEwMDY0NAotLS0gYS9X ZWJDb3JlL2RvbS9EZXZpY2VPcmllbnRhdGlvbkV2ZW50LmgKKysrIGIvV2ViQ29yZS9kb20vRGV2 aWNlT3JpZW50YXRpb25FdmVudC5oCkBAIC0yNiwxMiArMjYsMTEgQEAKICNpZm5kZWYgRGV2aWNl T3JpZW50YXRpb25FdmVudF9oCiAjZGVmaW5lIERldmljZU9yaWVudGF0aW9uRXZlbnRfaAogCisj aW5jbHVkZSAiRGV2aWNlT3JpZW50YXRpb24uaCIKICNpbmNsdWRlICJFdmVudC5oIgogCiBuYW1l c3BhY2UgV2ViQ29yZSB7CiAKLWNsYXNzIERldmljZU9yaWVudGF0aW9uOwotCiBjbGFzcyBEZXZp Y2VPcmllbnRhdGlvbkV2ZW50IDogcHVibGljIEV2ZW50IHsKIHB1YmxpYzoKICAgICBzdGF0aWMg UGFzc1JlZlB0cjxEZXZpY2VPcmllbnRhdGlvbkV2ZW50PiBjcmVhdGUoKQo=