Bug 171363 - Web Inspector: ASSERT(!m_startedComposite) fails when recording on non-Cocoa Timeline
Summary: Web Inspector: ASSERT(!m_startedComposite) fails when recording on non-Cocoa ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-26 20:04 PDT by Ross Kirsling
Modified: 2017-05-01 16:54 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.36 KB, patch)
2017-04-26 20:18 PDT, Ross Kirsling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ross Kirsling 2017-04-26 20:04:10 PDT
It appears that when compositing events were implemented, this wasn't done in an entirely safe fashion for non-Cocoa platforms:
https://bugs.webkit.org/show_bug.cgi?id=146168

Specifically, the asserts for InspectorTimelineAgent::willComposite and InspectorTimelineAgent::didComposite ensure that one is not used without the other, yet didComposite is called within a PLATFORM(COCOA) block while willComposite is called on all platforms.

Asserts:                https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/inspector/InspectorTimelineAgent.cpp#L358
didComposite callsite:  https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/inspector/InspectorTimelineAgent.cpp#L196
willComposite callsite: https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/page/FrameView.cpp#L1234

As such, on any non-Cocoa platform, the second compositing event in a timeline recording causes an assertion failure.
Comment 1 Ross Kirsling 2017-04-26 20:04:24 PDT
Compositing events are Cocoa-specific until we have platform abstraction for RunLoopObserver:
https://bugs.webkit.org/show_bug.cgi?id=142748

Therefore we should simply guard the willComposite callsite for now.
Comment 2 Ross Kirsling 2017-04-26 20:18:37 PDT
Created attachment 308327 [details]
Patch
Comment 3 BJ Burg 2017-04-27 16:39:57 PDT
I'd like Matt to review this when he's back in office next week.
Comment 4 Matt Baker 2017-05-01 16:25:21 PDT
Comment on attachment 308327 [details]
Patch

r=me.

I don't have an easy way to test this, but it seems clear that the second time willComposite is called (and every time after) it will assert, since didComposite is never called. Ross's patch matches the guards around the creation of the run loop observation points and looks correct.
Comment 5 WebKit Commit Bot 2017-05-01 16:54:34 PDT
Comment on attachment 308327 [details]
Patch

Clearing flags on attachment: 308327

Committed r216043: <http://trac.webkit.org/changeset/216043>
Comment 6 WebKit Commit Bot 2017-05-01 16:54:35 PDT
All reviewed patches have been landed.  Closing bug.