Bug 108612 – [WK2][Win] MessageID.h related changes broke the build

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zoltan Arvai]

In range of r141459 and r141528 there were several changes in CoreIPC that broke Qt windows build.
Probably its related to r141465, r141472, r141528 and 141538.

http://build.webkit.org/builders/Qt%20Windows%2032-bit%20Release/builds/61228/steps/compile-webkit/logs/stdio

C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(99) : error C2039: 'unregisterAndCloseHandle' : is not a member of 'WTF::RefPtr<T>'
        with
        [
            T=WorkQueue
        ]
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(102) : error C2039: 'unregisterAndCloseHandle' : is not a member of 'WTF::RefPtr<T>'
        with
        [
            T=WorkQueue
        ]
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(171) : error C2065: 'MessageID' : undeclared identifier
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(171) : error C2070: ''unknown-type'': illegal sizeof operand
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(174) : error C3861: 'MessageID': identifier not found
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(263) : error C2039: 'registerHandle' : is not a member of 'WTF::RefPtr<T>'
        with
        [
            T=WorkQueue
        ]
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(264) : error C2039: 'registerHandle' : is not a member of 'WTF::RefPtr<T>'
        with
        [
            T=WorkQueue
        ]
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(267) : error C2039: 'dispatch' : is not a member of 'WTF::RefPtr<T>'
        with
        [
            T=WorkQueue
        ]
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(280) : error C2065: 'MessageID' : undeclared identifier
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(280) : error C2146: syntax error : missing ')' before identifier 'messageID'
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(280) : error C2761: 'bool CoreIPC::Connection::sendOutgoingMessage(WTF::PassOwnPtr<T>)' : member function redeclaration not allowed
        with
        [
            T=CoreIPC::MessageEncoder
        ]
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(280) : error C2059: syntax error : ')'
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(281) : error C2143: syntax error : missing ';' before '{'
C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebKit2\Platform\CoreIPC\win\ConnectionWin.cpp(281) : error C2447: '{' : missing function header (old-style formal list?)

[Zoltan Arvai]

Created attachment 186005 [details]
first attempt to fix

Trying to fix the build, more modifications needed as i see.

[Zoltan Arvai]

With the patch I got some other error messages:

C:\WebKitBuildSlave\proba\WebKit\Source\WebKit2\Platform\win\WorkQueueWin.cpp(147) : error C2065: 'm_isValidMutex' : undeclared identifier
C:\WebKitBuildSlave\proba\WebKit\Source\WebKit2\Platform\win\WorkQueueWin.cpp(148) : error C2065: 'm_isValid' : undeclared identifier

let's see....

[Simon Hausmann]

Comment on attachment 186005 [details]
first attempt to fix

View in context: https://bugs.webkit.org/attachment.cgi?id=186005&action=review

> Source/WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp:172
>              OwnPtr<MessageDecoder> decoder = MessageDecoder::create(DataReference(m_readBuffer.data(), realBufferSize));

I think you can just get rid of realBufferSize and use m_readBuffer.size() directly here. Without the ASSERT there I guess the comment doesn't add any value neither?

[Simon Hausmann]

Created attachment 186011 [details]
Prospective patch for WorkQueueWin.cpp

(In reply to comment #2)
> With the patch I got some other error messages:
> 
> C:\WebKitBuildSlave\proba\WebKit\Source\WebKit2\Platform\win\WorkQueueWin.cpp(147) : error C2065: 'm_isValidMutex' : undeclared identifier
> C:\WebKitBuildSlave\proba\WebKit\Source\WebKit2\Platform\win\WorkQueueWin.cpp(148) : error C2065: 'm_isValid' : undeclared identifier
> 
> let's see....

The attached patch should fix that part, I think.

[Zoltan Arvai]

Build is seems fine with MSVC. I'll create the aggregated patch for buildfix soon :)

[Zoltan Arvai]

Created attachment 186031 [details]
patch for landing

Aggregated patch, builds with MSVC.

[Simon Hausmann]

Comment on attachment 186031 [details]
patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=186031&action=review

> Source/WebKit2/ChangeLog:2
> +        [Qt][Win] Fix build after MessageID.h related changes.

Newline missing before this line?

[Zoltan Arvai]

Created attachment 186039 [details]
patch for landing

corrected patch, sorry

[Simon Hausmann]

This patch doesn't touch any Qt specific files, so replacing [Qt] in the bug title with [WK2] :)

[Zoltan Arvai]

Created attachment 186043 [details]
patch for landing

And sorry again. Replacing Qt with WK2 in changelog, too. I see Qt everywhere :)

[Anders Carlsson]

Comment on attachment 186043 [details]
patch for landing

I think it's better if WorkItemWin just used a RefPtr instead of manually reffing and dereffing.

[Zoltan Arvai]

Is that possible, that the current WorkQueueWin.cpp implementation has the required RefPtr functionality and doing ref - deref was not necessary in the last patch?

http://trac.webkit.org/browser/trunk/Source/WebKit2/Platform/win/WorkQueueWin.cpp

[Anders Carlsson]

(In reply to comment #12)
> Is that possible, that the current WorkQueueWin.cpp implementation has the required RefPtr functionality and doing ref - deref was not necessary in the last patch?
> 
> http://trac.webkit.org/browser/trunk/Source/WebKit2/Platform/win/WorkQueueWin.cpp

No, that is not possible:

    class WorkItemWin : public ThreadSafeRefCounted<WorkItemWin> {
    public:
        static PassRefPtr<WorkItemWin> create(const Function<void()>&, WorkQueue*);
        virtual ~WorkItemWin();

        Function<void()>& function() { return m_function; }
        WorkQueue* queue() const { return m_queue; }

    protected:
        WorkItemWin(const Function<void()>&, WorkQueue*);

    private:
        Function<void()> m_function;
        WorkQueue* m_queue;
    };

m_queue needs to be a RefPtr.

[Zoltan Arvai]

Created attachment 186649 [details]
patch

Build fix for QtWin using RefPtr. 
Including fix for changes in r141619 (based on r141632 Qt fix).

[Anders Carlsson]

Comment on attachment 186649 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=186649&action=review

> Source/WebKit2/Platform/WorkQueue.h:106
> +        PassRefPtr<WorkQueue> queue() const { return m_queue; }

This should just return a raw pointer. Patch looks good otherwise!

[Zoltan Arvai]

Created attachment 186667 [details]
corrected patch

Correcting raw pointer return. Thanks.

[Build Bot]

Comment on attachment 186667 [details]
corrected patch

Attachment 186667 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16378790

[Zoltan Arvai]

Is the Win EWS broken?
"Updating working directory
Failed to run "['Tools/Scripts/update-webkit']" exit_code: 1"

[Csaba Osztrogonác]

(In reply to comment #18)
> Is the Win EWS broken?
> "Updating working directory
> Failed to run "['Tools/Scripts/update-webkit']" exit_code: 1"

Who cares the Apple Win EWS here? Your patch touches only WK2 
files, but WK2 isn't supported by Apple Windows port. This
change can affect only Qt-WK2 Windows port.

[WebKit Review Bot]

Comment on attachment 186667 [details]
corrected patch

Clearing flags on attachment: 186667

Committed r142002: <http://trac.webkit.org/changeset/142002>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Gyuyoung Kim]

*** Bug 108985 has been marked as a duplicate of this bug. ***