Bug 133664 – [iOS] Clear UIProcess visual state after WebProcess crash

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

[Martin Hock]

[iOS] Clear UIProcess visual state after WebProcess crash

[Martin Hock]

Created attachment 232747 [details]
patch

[Martin Hock]

<rdar://problem/16952742>

[Benjamin Poulain]

Comment on attachment 232747 [details]
patch

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:641
> +    _overridesMinimumLayoutSize = NO;
> +    _minimumLayoutSizeOverride = {0, 0};
> +    _overridesMinimumLayoutSizeForMinimalUI = NO;
> +    _minimumLayoutSizeOverrideForMinimalUI = {0, 0};
> +    _overridesMaximumUnobscuredSize = NO;
> +    _maximumUnobscuredSizeOverride = {0, 0};
> +    _usesMinimalUI = NO;
> +    _needsToNotifyDelegateAboutMinimalUI = NO;
> +    _inputViewBounds = {{0, 0}, {0, 0}};

Most of those are defined by external factors, they should not be reset on crash!

> Source/WebKit2/UIProcess/mac/RemoteLayerTreeHost.mm:147
> +    for (auto& idLayer : m_layers) {
> +        m_animationDelegates.remove(idLayer.key);
> +        asLayer(idLayer.value.get()).contents = nullptr;
> +    }

Shouldn't we remove the sublayers before clearing the root?

[Martin Hock]

Created attachment 232898 [details]
address comments

[Martin Hock]

(In reply to comment #3)
> (From update of attachment 232747 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232747&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:641
> > +    _overridesMinimumLayoutSize = NO;
> > +    _minimumLayoutSizeOverride = {0, 0};
> > +    _overridesMinimumLayoutSizeForMinimalUI = NO;
> > +    _minimumLayoutSizeOverrideForMinimalUI = {0, 0};
> > +    _overridesMaximumUnobscuredSize = NO;
> > +    _maximumUnobscuredSizeOverride = {0, 0};
> > +    _usesMinimalUI = NO;
> > +    _needsToNotifyDelegateAboutMinimalUI = NO;
> > +    _inputViewBounds = {{0, 0}, {0, 0}};
> 
> Most of those are defined by external factors, they should not be reset on crash!

Yes, I should be pushing instead. Fixed.

> > Source/WebKit2/UIProcess/mac/RemoteLayerTreeHost.mm:147
> > +    for (auto& idLayer : m_layers) {
> > +        m_animationDelegates.remove(idLayer.key);
> > +        asLayer(idLayer.value.get()).contents = nullptr;
> > +    }
> 
> Shouldn't we remove the sublayers before clearing the root?

I followed the same order as updateLayerTree which implies that the root is updated before the layers, but changed this anyway as the custom is to destroy in opposite order of creation.

[Benjamin Poulain]

Comment on attachment 232898 [details]
address comments

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

> Source/WebKit2/UIProcess/ios/WKContentView.mm:387
> +    [_webView _didRelaunchProcess];

You should do this from the iOS's PageClient instead. No need to push ContentView->WebView here.

[Martin Hock]

Created attachment 232908 [details]
address comment + fix mac build

[Martin Hock]

Committed r169851: <http://trac.webkit.org/changeset/169851>