Bug 141037 – Convert WebInspectorProxy to use WKWebView API for the inspector view

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brian Burg]

.

[Brian Burg]

Created attachment 245656 [details]
WIP

[Brian Burg]

Created attachment 245657 [details]
WIP

[Brian Burg]

The attached patch mostly works, except for dock to bottom. In that case, the initial draw seems to be correct, but subsequent draws paint the inspector over the entire inspected page superview, with web content on top.

Our initial assessment is that Safari or some other component is resizing the view behind our backs. Not sure I can diagnose further from OpenSource only, so benching this patch for now.

There is also some clean up to do with the PageGroup stuff. Unless I'm missing some important detail, the whole "level" system can be significantly simplified now that multi-process guarantees concurrent execution between different inspector levels (no stalling during debugger pauses).

[Sam Weinig]

(In reply to comment #3)
> The attached patch mostly works, except for dock to bottom. In that case,
> the initial draw seems to be correct, but subsequent draws paint the
> inspector over the entire inspected page superview, with web content on top.
> 
> Our initial assessment is that Safari or some other component is resizing
> the view behind our backs. Not sure I can diagnose further from OpenSource
> only, so benching this patch for now.

Do you see the same behavior in MiniBrowser? If so, I would recommend debugging from there.

> 
> There is also some clean up to do with the PageGroup stuff. Unless I'm
> missing some important detail, the whole "level" system can be significantly
> simplified now that multi-process guarantees concurrent execution between
> different inspector levels (no stalling during debugger pauses).

I am generally confused by the layering of this code.  It seems like the inspector code in WebKit2 lives both at the WebPageProxy/C++ layer, yet knows about things like WKWebView.  Is there something cleaner we can do here?

[Brian Burg]

> > There is also some clean up to do with the PageGroup stuff. Unless I'm
> > missing some important detail, the whole "level" system can be significantly
> > simplified now that multi-process guarantees concurrent execution between
> > different inspector levels (no stalling during debugger pauses).
> 
> I am generally confused by the layering of this code.  It seems like the
> inspector code in WebKit2 lives both at the WebPageProxy/C++ layer, yet
> knows about things like WKWebView.  Is there something cleaner we can do
> here?

Most of inspector WK2 code is cross-platform and in the C++ layer, except port-specific WebInspectorProxy functions. The platform-specific parts are to manage frame sizes and windows while attached and detached. This isn't cross-platform because every port does windowing differently.

I think it is pretty well segregated as-is; this patch does not affect ports other than Mac. Other code in WebInspectorProxy and WebKit2 doesn't care whether WKView or WKWebView API is used to set up the underlying WebPageProxy.

[Timothy Hatcher]

Disabling automaticallyAdjustsContentInsets fixes this.

Add this after creating the WKWebView in m_inspectorView:

#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
    [m_inspectorView _setAutomaticallyAdjustsContentInsets:NO];
#endif

[Brian Burg]

Created attachment 245817 [details]
Patch

[Timothy Hatcher]

Comment on attachment 245817 [details]
Patch

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

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:498
>      [m_inspectorView setDrawsBackground:NO];

This needs to be _setDrawsTransparentBackground:YES to fix the 10.9 build.

[Brian Burg]

Created attachment 245836 [details]
Patch for EWS

[Timothy Hatcher]

Comment on attachment 245836 [details]
Patch for EWS

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

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:497
> -#if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
> -    [m_inspectorView setDrawsBackground:NO];
> +    [m_inspectorView _setDrawsTransparentBackground:YES];

On Yosemite we don't need to be transparent. We draw everything. So the __MAC_OS_X_VERSION_MIN_REQUIRED <= 1090 check needs to stay.

[Timothy Hatcher]

Comment on attachment 245836 [details]
Patch for EWS

Just add that #ifdef around _setDrawsTransparentBackground:YES back and this looks good to me.

[Brian Burg]

Committed r179540: <http://trac.webkit.org/changeset/179540>

[Darin Adler]

After this landed, 32-bit build started failing:

https://build.webkit.org/builders/Apple%20Yosemite%20Release%20%2832-bit%20Build%29/builds/270/steps/compile-webkit/logs/stdio

[WebKit Commit Bot]

Re-opened since this is blocked by bug 141190

[Brian Burg]

Created attachment 245961 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 245961 [details]
Patch

Clearing flags on attachment: 245961

Committed r179573: <http://trac.webkit.org/changeset/179573>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.