Bug 112445

Summary: Support connecting the Web Inspector without showing it
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: WebKit2Assignee: Timothy Hatcher <timothy>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, andersca, cmarcelo, graouts, gyuyoung.kim, joepeck, menard, rakuco, rniwa, sam, timothy, webkit-bug-importer, webkit-ews, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Proposed Change
none
Proposed Change (Take 2)
none
Proposed Change (Take 3)
sam: review-, webkit-ews: commit-queue-
Proposed Change (Take 4)
webkit-ews: commit-queue-
Proposed Change (Take 5)
webkit-ews: commit-queue-
Proposed Change (Take 6) none

Description Timothy Hatcher 2013-03-15 07:30:23 PDT
We should support connecting the Web Inspector without showing the window or attaching. This would allow you to get the debugger attached in the background. When a breakpoint is hit the Inspector will open. You should also be able hide the Inspector and keep it connected.

These options will not the be default behavior, but an option in menus.
Comment 1 Timothy Hatcher 2013-03-15 07:46:24 PDT
Created attachment 193307 [details]
Proposed Change
Comment 2 Radar WebKit Bug Importer 2013-03-15 07:48:50 PDT
<rdar://problem/13430045>
Comment 3 Timothy Hatcher 2013-03-15 08:20:18 PDT
Created attachment 193314 [details]
Proposed Change (Take 2)
Comment 4 Joseph Pecoraro 2013-03-15 23:23:24 PDT
Comment on attachment 193314 [details]
Proposed Change (Take 2)

Looks good to me.
Comment 5 Timothy Hatcher 2013-03-16 06:52:46 PDT
Anders, Sam, want to do a quick review of this?
Comment 6 Timothy Hatcher 2013-03-16 10:23:31 PDT
Created attachment 193437 [details]
Proposed Change (Take 3)

I've simplified some things since the last patch.
Comment 7 Early Warning System Bot 2013-03-16 10:33:30 PDT
Comment on attachment 193437 [details]
Proposed Change (Take 3)

Attachment 193437 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17188423
Comment 8 EFL EWS Bot 2013-03-16 10:35:13 PDT
Comment on attachment 193437 [details]
Proposed Change (Take 3)

Attachment 193437 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17222202
Comment 9 Sam Weinig 2013-03-16 19:30:50 PDT
Comment on attachment 193437 [details]
Proposed Change (Take 3)

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

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:413
> +        WKView *inspectedView = m_page->wkView();

Please don't access the WKView directly here (that function should be removed).  Instead, this should go through the PageClient.
Comment 10 Timothy Hatcher 2013-03-20 12:57:26 PDT
Comment on attachment 193437 [details]
Proposed Change (Take 3)

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

>> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:413
>> +        WKView *inspectedView = m_page->wkView();
> 
> Please don't access the WKView directly here (that function should be removed).  Instead, this should go through the PageClient.

This isn't new to this patch, we use wkView in a couple other places in this file. PageClient isn't exposed on WebPageProxy right now. I'd like to keep this patch as-is. I filed bug 112829 to track that.
Comment 11 Timothy Hatcher 2013-03-20 13:11:42 PDT
Created attachment 194102 [details]
Proposed Change (Take 4)
Comment 12 Early Warning System Bot 2013-03-20 13:34:33 PDT
Comment on attachment 194102 [details]
Proposed Change (Take 4)

Attachment 194102 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17258038
Comment 13 Timothy Hatcher 2013-03-20 13:41:54 PDT
Created attachment 194108 [details]
Proposed Change (Take 5)
Comment 14 Early Warning System Bot 2013-03-20 14:12:25 PDT
Comment on attachment 194108 [details]
Proposed Change (Take 5)

Attachment 194108 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17253236
Comment 15 Timothy Hatcher 2013-03-20 14:17:02 PDT
Created attachment 194122 [details]
Proposed Change (Take 6)
Comment 16 Timothy Hatcher 2013-03-21 09:17:41 PDT
Sam or Anders, mind taking a look at take 6?
Comment 17 WebKit Review Bot 2013-03-21 14:19:04 PDT
Comment on attachment 194122 [details]
Proposed Change (Take 6)

Clearing flags on attachment: 194122

Committed r146518: <http://trac.webkit.org/changeset/146518>
Comment 18 WebKit Review Bot 2013-03-21 14:19:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 19 Ryosuke Niwa 2013-03-21 19:26:45 PDT
This patch appears to have broken two tests on ML Debug testers:
e.g.
http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20(Tests)/r146520%20(8040)/results.html
Comment 20 Ryosuke Niwa 2013-03-22 01:23:09 PDT
This patch may have caused inspector/profiler/cpu-profiler-profile-removal.html to intermittently crash. See:
https://bugs.webkit.org/show_bug.cgi?id=113020
Comment 21 Timothy Hatcher 2013-03-22 05:17:30 PDT
These are ASSERTs. I am looking into it.