Bug 126923 – Web Inspector: support handling mouse events on the inspector page overlay

• Component: Web Inspector
• Status: NEW

[Blaze Burg]

This will allow interaction with the overlay, such as showing interactive highlights of shapes, regions, rectangle selection, etc.

[Timothy Hatcher]

<rdar://problem/15806447>

[Brian Burg]

Created attachment 245608 [details]
[WIP]

[Brian Burg]

WIP notes:

Current idea is to add a WKWebView for the overlay page rather than using a PageOverlay. Specific highlight methods can be moved over to this overlay as part of the patch or in a follow-up.

As for page lifetime, the patch pairs together the setup and teardown of the overlay page and the inspector page. The overlay page is not used unless the inspector is active. (This may be split into two messages if necessary to make IPC work out.)

The point of making this overlay a real WebPage is so that we can add buttons, tooltips, direct manipulation, and other interactive features without further shoehorning PageOverlay to do things it wasn't designed for.

To me, it doesn't make much sense to put iOS (i.e., CALayer based) paint highlighting through this overlay page because 1) paint flashing requires low latency, which in practice is hard given the message-passing nature of Inspector and Overlay pages; 2) paint flashes aren't interactive, so they get no benefit from being rendered to an interactive overlay page.

As for other ports, I assume they can easily create an overlay page and composite it over the inspected page when the inspector opens. That's what a WKWebView approach does for mac/ios.

[Brian Burg]

Created attachment 245844 [details]
WIP

[Timothy Hatcher]

Comment on attachment 245844 [details]
WIP

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

> Source/WebKit2/UIProcess/WebInspectorProxy.h:138
> +    static bool isOverlayPage(WebPageProxy&);

isInspectorOverlayPage(WebPageProxy&)?

[Brian Burg]

Created attachment 246101 [details]
WIP

Latest patch can show and hide the overlay WKWebView. Does not forward unhandled user inputs yet. Does not recieve highlight commands.

[Timothy Hatcher]

Comment on attachment 246101 [details]
WIP

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

> Source/WebCore/inspector/InspectorDOMAgent.h:112
> +    virtual void didCreateFrontendAndBackend(Inspector::InspectorFrontendChannel*, Inspector::OverlayChannel*, Inspector::InspectorBackendDispatcher*) override;

We should rename Inspector::InspectorFrontendChannel to something like Inspector::FrontendChannel or Inspector::UserInterfaceChannel.

> Source/WebKit2/UIProcess/WebInspectorProxy.cpp:502
> +void WebInspectorProxy::eagerlyCreateOverlayPage()

Not sure we need to eagerly do this for the overlay. I did it for the fronted because it shaves ~500ms of load time opening the Inspector. Though if there is a noticeable delay in the overlay, maybe we should.

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:947
> +    [[NSNotificationCenter defaultCenter] addObserver:m_inspectorProxyObjCAdapter.get() selector:@selector(inspectedViewFrameDidChange:) name:NSViewFrameDidChangeNotification object:inspectedView];

We should consider using constraints here (and for the Inspector). They will allow us to auto magically follow the frame. I plan to look at this soon (for other reasons) if you don't want to try it. It might require switching other views to auto layout and sprinkling translatesAutoresizingMaskIntoConstraints = NO.

Roughly what you should need:

    [NSLayoutConstraint constraintWithItem:overlayView attribute:NSLayoutAttributeTop relatedBy:NSLayoutRelationEqual toItem:inspectedView attribute:NSLayoutAttributeTop multiplier:1 constant:0].active = YES;
    [NSLayoutConstraint constraintWithItem:overlayView attribute:NSLayoutAttributeBottom relatedBy:NSLayoutRelationEqual toItem:inspectedView attribute:NSLayoutAttributeBottom multiplier:1 constant:0].active = YES;
    [NSLayoutConstraint constraintWithItem:overlayView attribute:NSLayoutAttributeLeft relatedBy:NSLayoutRelationEqual toItem:inspectedView attribute:NSLayoutAttributeLeft multiplier:1 constant:0].active = YES;
    [NSLayoutConstraint constraintWithItem:overlayView attribute:NSLayoutAttributeRight relatedBy:NSLayoutRelationEqual toItem:inspectedView attribute:NSLayoutAttributeRight multiplier:1 constant:0].active = YES;

[Brian Burg]

Comment on attachment 246101 [details]
WIP

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

>> Source/WebCore/inspector/InspectorDOMAgent.h:112
>> +    virtual void didCreateFrontendAndBackend(Inspector::InspectorFrontendChannel*, Inspector::OverlayChannel*, Inspector::InspectorBackendDispatcher*) override;
> 
> We should rename Inspector::InspectorFrontendChannel to something like Inspector::FrontendChannel or Inspector::UserInterfaceChannel.

In general, this seems fine for things defined in Inspector namespace within JSC. For WebCore things, it might lead to some awkward filename clashes.

>> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:947
>> +    [[NSNotificationCenter defaultCenter] addObserver:m_inspectorProxyObjCAdapter.get() selector:@selector(inspectedViewFrameDidChange:) name:NSViewFrameDidChangeNotification object:inspectedView];
> 
> We should consider using constraints here (and for the Inspector). They will allow us to auto magically follow the frame. I plan to look at this soon (for other reasons) if you don't want to try it. It might require switching other views to auto layout and sprinkling translatesAutoresizingMaskIntoConstraints = NO.
> 
> Roughly what you should need:
> 
>     [NSLayoutConstraint constraintWithItem:overlayView attribute:NSLayoutAttributeTop relatedBy:NSLayoutRelationEqual toItem:inspectedView attribute:NSLayoutAttributeTop multiplier:1 constant:0].active = YES;
>     [NSLayoutConstraint constraintWithItem:overlayView attribute:NSLayoutAttributeBottom relatedBy:NSLayoutRelationEqual toItem:inspectedView attribute:NSLayoutAttributeBottom multiplier:1 constant:0].active = YES;
>     [NSLayoutConstraint constraintWithItem:overlayView attribute:NSLayoutAttributeLeft relatedBy:NSLayoutRelationEqual toItem:inspectedView attribute:NSLayoutAttributeLeft multiplier:1 constant:0].active = YES;
>     [NSLayoutConstraint constraintWithItem:overlayView attribute:NSLayoutAttributeRight relatedBy:NSLayoutRelationEqual toItem:inspectedView attribute:NSLayoutAttributeRight multiplier:1 constant:0].active = YES;

Sounds fine to me (it might allow us to remove the ObjC proxy), though I probably won't have time to attempt that myself. The frame sizing code in this file is kinda scary.

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:950
> +    // FIXME: when the overlay is ready to handle user input, make first responder here.

Am still investigating how Cocoa or WebKit decides which WebPageProxy to send user input to. Without first responder here, the overlay still gets most events.

[Brian Burg]

Created attachment 246388 [details]
Patch

Latest patch implements overlay attach and detach. Does not yet delegate events to inspected page when not inactive.