Bug 79279 - Synchronous ShouldGoToBackForwardListItem causes lots of WebProcess hangs
Summary: Synchronous ShouldGoToBackForwardListItem causes lots of WebProcess hangs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-02-22 14:39 PST by Brady Eidson
Modified: 2012-02-22 16:16 PST (History)
0 users

See Also:


Attachments
Patch v1 (13.73 KB, patch)
2012-02-22 16:01 PST, Brady Eidson
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2012-02-22 14:39:36 PST
Synchronous ShouldGoToBackForwardListItem causes lots of WebProcess hangs

This WK2 API was somewhat blindly carried over from WebKit1.

Unfortunately it's one of a handful of synchronous messages from the WebProcess to the UI Process that causes hangs and seems unnecessary. 

It seems rare that a client would want to answer "no" to this call, so making it an asynchronous, notification-only callback would clear those hangs!

A patch implementing "WillGoToBackForwardListItem" in the version 1 loader client is coming soon.

If clients do decide that it'd be important to implement an actual "should" policy callback, it should be done in the bundle client and we can introduce that when it's needed.

In radar as <rdar://problem/10406044>
Comment 1 Brady Eidson 2012-02-22 16:01:37 PST
Created attachment 128315 [details]
Patch v1
Comment 2 Anders Carlsson 2012-02-22 16:04:23 PST
Comment on attachment 128315 [details]
Patch v1

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

> Source/WebKit2/UIProcess/WebLoaderClient.cpp:245
> +    if (m_client.version > 0 || !m_client.shouldGoToBackForwardListItem)
>          return true;

You should add a comment here explaining the version check.

> Source/WebKit2/WebProcess/WebPage/WebPage.h:506
> +    bool willGoToBackForwardItemCallbackEnabled() { return m_willGoToBackForwardItemCallbackEnabled; }

This can be const.
Comment 3 Brady Eidson 2012-02-22 16:16:56 PST
http://trac.webkit.org/changeset/108570