Bug 133664 - [iOS] Clear UIProcess visual state after WebProcess crash
Summary: [iOS] Clear UIProcess visual state after WebProcess crash
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Hock
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-09 16:38 PDT by Martin Hock
Modified: 2014-06-11 15:50 PDT (History)
2 users (show)

See Also:


Attachments
patch (3.52 KB, patch)
2014-06-09 16:40 PDT, Martin Hock
benjamin: review-
Details | Formatted Diff | Diff
address comments (5.04 KB, patch)
2014-06-11 13:56 PDT, Martin Hock
benjamin: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff
address comment + fix mac build (4.90 KB, patch)
2014-06-11 14:53 PDT, Martin Hock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Hock 2014-06-09 16:38:26 PDT
[iOS] Clear UIProcess visual state after WebProcess crash
Comment 1 Martin Hock 2014-06-09 16:40:49 PDT
Created attachment 232747 [details]
patch
Comment 2 Martin Hock 2014-06-09 16:42:11 PDT
<rdar://problem/16952742>
Comment 3 Benjamin Poulain 2014-06-09 19:25:42 PDT
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?
Comment 4 Martin Hock 2014-06-11 13:56:20 PDT
Created attachment 232898 [details]
address comments
Comment 5 Martin Hock 2014-06-11 13:58:58 PDT
(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.
Comment 6 Benjamin Poulain 2014-06-11 14:25:36 PDT
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.
Comment 7 Martin Hock 2014-06-11 14:53:12 PDT
Created attachment 232908 [details]
address comment + fix mac build
Comment 8 Martin Hock 2014-06-11 15:50:09 PDT
Committed r169851: <http://trac.webkit.org/changeset/169851>