Bug 144080 – [iOS] Disable live rendering during zoom on low-memory devices.

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Layout and Rendering

[Andreas Kling]

<rdar://problem/19872709>

To avoid IOSurface explosion on low-memory (512MB) devices, we should disable live tile rendering during zooming.

[Andreas Kling]

Created attachment 251386 [details]
Patch idea

[Simon Fraser (smfr)]

Comment on attachment 251386 [details]
Patch idea

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:158
> +static uint64_t memorySize()
> +{
> +    static host_basic_info_data_t hostInfo;
> +
> +    static dispatch_once_t once;
> +    dispatch_once(&once, ^() {
> +        mach_port_t host = mach_host_self();
> +        mach_msg_type_number_t count = HOST_BASIC_INFO_COUNT;
> +        kern_return_t r = host_info(host, HOST_BASIC_INFO, (host_info_t)&hostInfo, &count);
> +        mach_port_deallocate(mach_task_self(), host);
> +
> +        if (r != KERN_SUCCESS)
> +            LOG_ERROR("%s : host_info(%d) : %s.\n", __FUNCTION__, r, mach_error_string(r));
> +    });
> +
> +    return hostInfo.max_mem;
> +}

There are already 3 copies of this code. Please don't add another one.

[Tim Horton]

Comment on attachment 251386 [details]
Patch idea

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

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:142
> +static uint64_t memorySize()

Put this somewhere we can share it. Probably in WTF. Certainly does not belong in WKWebView.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1382
> +            drawingArea->setLayerTreeStateIsFrozen(true);

This is kind of a weird way to do this. I feel like the Web process should be able to do this without help (?). Also, isn't there lots of wasted work happening in the Web process in your case, because you're only cutting off the layer tree flush, but not other things? Maybe Ben has an idea.

[Andreas Kling]

(In reply to comment #3)
> Comment on attachment 251386 [details]
> Patch idea
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=251386&action=review
> 
> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:142
> > +static uint64_t memorySize()
> 
> Put this somewhere we can share it. Probably in WTF. Certainly does not
> belong in WKWebView.

Sure. I'll consolidate all the existing ones in a separate patch. Filed this:
<https://webkit.org/b/144081> WTF should have a way to get the system memory size.

> > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1382
> > +            drawingArea->setLayerTreeStateIsFrozen(true);
> 
> This is kind of a weird way to do this. I feel like the Web process should
> be able to do this without help (?). Also, isn't there lots of wasted work
> happening in the Web process in your case, because you're only cutting off
> the layer tree flush, but not other things? Maybe Ben has an idea.

This approach came out of a discussion with Ben. We wanted something that still behaves the same, as far as the web content is concerned (scroll events still get sent, and such) and just looks different.

Would you like it better if the IPC messages were something like "WillBeginZooming/DidEndZooming" instead, and the biz logic for what to do about it would sit in the web process?

[Tim Horton]

(In reply to comment #4)
> (In reply to comment #3)
> > Comment on attachment 251386 [details]
> > Patch idea
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=251386&action=review
> > 
> > > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:142
> > > +static uint64_t memorySize()
> > 
> > Put this somewhere we can share it. Probably in WTF. Certainly does not
> > belong in WKWebView.
> 
> Sure. I'll consolidate all the existing ones in a separate patch. Filed this:
> <https://webkit.org/b/144081> WTF should have a way to get the system memory
> size.

ramSize() already exists, as mentioned on IRC.

> > > Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1382
> > > +            drawingArea->setLayerTreeStateIsFrozen(true);
> > 
> > This is kind of a weird way to do this. I feel like the Web process should
> > be able to do this without help (?). Also, isn't there lots of wasted work
> > happening in the Web process in your case, because you're only cutting off
> > the layer tree flush, but not other things? Maybe Ben has an idea.
> 
> This approach came out of a discussion with Ben. We wanted something that
> still behaves the same, as far as the web content is concerned (scroll
> events still get sent, and such) and just looks different.

Ah! So the weird behavior is intentional :D OK.

> Would you like it better if the IPC messages were something like
> "WillBeginZooming/DidEndZooming" instead, and the biz logic for what to do
> about it would sit in the web process?

That does sound better. I'm still sort of surprised that the Web process can't already tell when live zooming starts and ends, but perhaps it cannot.

Discussing on IRC why setLayerTreeStateIsFrozen is scary.