Bug 42447 – DeviceOrientationEvent.h should not forward-declare DeviceOrientation

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Hans Wennborg]

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;

[Hans Wennborg]

Created attachment 61795 [details]
Patch

[Steve Block]

Comment on attachment 61795 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 61795 [details]
Patch

Clearing flags on attachment: 61795

Committed r63544: <http://trac.webkit.org/changeset/63544>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

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.

[Hans Wennborg]

(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?

[Hans Wennborg]

Uploaded patch to Bug 42466 that implements Darin's suggestion, as it reduces the header size.