Bug 200125 - Web Inspector: Timelines: Develop > Start Timeline Recording doesn't work when focused on a detached inspector window
Summary: Web Inspector: Timelines: Develop > Start Timeline Recording doesn't work whe...
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: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-07-25 08:20 PDT by Devin Rousso
Modified: 2019-08-02 13:39 PDT (History)
7 users (show)

See Also:


Attachments
Patch (20.68 KB, patch)
2019-07-25 16:08 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff
Patch (20.78 KB, patch)
2019-08-02 12:54 PDT, Devin Rousso
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Devin Rousso 2019-07-25 08:20:04 PDT
# STEPS TO REPRODUCE:
1. inspect any page
2. close/"remove" the Timelines tab
3. detach Web Inspector
4. Develop > Start Timeline Recording
 => nothing happens (if the page is focused instead of the detached inspector window, the Timelines tab is re-"added" and works as expected)
Comment 1 Radar WebKit Bug Importer 2019-07-25 08:20:24 PDT
<rdar://problem/53543008>
Comment 2 Devin Rousso 2019-07-25 16:08:03 PDT
Created attachment 374920 [details]
Patch
Comment 3 Brian Burg 2019-08-02 11:18:11 PDT
Comment on attachment 374920 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm:-115
> -- (void)showTimelines

This does appear to be dead code once we remove the Safari call and rely on togglePageProfiling instead.

> Source/WebKit/UIProcess/WebInspectorProxy.cpp:350
>      m_isProfilingPage = !m_isProfilingPage;

Is this flag flipping now unnecessary, since we'll get notification from WebProcess once it actually works? If recording fails to start for some reason, this would erroneously think the timeline recording is active when it is not.
Comment 4 Devin Rousso 2019-08-02 12:36:47 PDT
Comment on attachment 374920 [details]
Patch

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

>> Source/WebKit/UIProcess/API/Cocoa/_WKInspector.mm:-115
>> -- (void)showTimelines
> 
> This does appear to be dead code once we remove the Safari call and rely on togglePageProfiling instead.

Yup!

>> Source/WebKit/UIProcess/WebInspectorProxy.cpp:350
>>      m_isProfilingPage = !m_isProfilingPage;
> 
> Is this flag flipping now unnecessary, since we'll get notification from WebProcess once it actually works? If recording fails to start for some reason, this would erroneously think the timeline recording is active when it is not.

Good point! =D
Comment 5 Devin Rousso 2019-08-02 12:54:55 PDT
Created attachment 375442 [details]
Patch
Comment 6 WebKit Commit Bot 2019-08-02 13:39:27 PDT
Comment on attachment 375442 [details]
Patch

Clearing flags on attachment: 375442

Committed r248177: <https://trac.webkit.org/changeset/248177>
Comment 7 WebKit Commit Bot 2019-08-02 13:39:29 PDT
All reviewed patches have been landed.  Closing bug.