Bug 70953 – Chromium ui_tests WorkerTest.WorkerMessagePort[GC] were broken by https://bugs.webkit.org/attachment.cgi?id=112342

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Dave Michael]

In http://trac.webkit.org/changeset/98381, I changed PlatformMessagePortChannel::hasPendingActivity thusly:
 bool PlatformMessagePortChannel::hasPendingActivity()
 {
     MutexLocker lock(m_mutex);
-    return m_localPort;
+    return m_localPort && m_localPort->hasPendingActivity();
 }

I suspect the new code is 'right' (in that I think it provides the originally _intended_ behavior), but existing code elsewhere must depend on the previous behavior.

I'm planning to undo this part of CS 98381, and hoping that somebody else more familiar with existing PlatformMessagePortChannels can fix the problem with this function and its caller(s) (if there is a problem).

[Dave Michael]

Created attachment 112572 [details]
Patch

[Dave Michael]

I verified locally that these ui tests pass with this change.

[David Levin]

Comment on attachment 112572 [details]
Patch

hmm. I missed that change somehow. It is best not to do unrelated changes in your patch and it is also good to explain why a change was necessary in the changelog where the function is listed.

I should have mentioned this in your previous change.

[David Levin]

Please re-enable the disabled tests where this gets into chromium.

(Also for changes of this sort, there should be some test to verify that the change is correct and stays in place, but that was for the last patch.)

[WebKit Review Bot]

Comment on attachment 112572 [details]
Patch

Clearing flags on attachment: 112572

Committed r98566: <http://trac.webkit.org/changeset/98566>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.