Bug 11675 - REGRESSION(r17871): Pages do not render properly when using the back/forward buttons in certain cases
Summary: REGRESSION(r17871): Pages do not render properly when using the back/forward ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P1 Major
Assignee: Brady Eidson
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2006-11-22 02:52 PST by Chris Beckinsale
Modified: 2006-12-07 08:18 PST (History)
6 users (show)

See Also:


Attachments
Darin and I hashed this one out (4.16 KB, patch)
2006-12-06 16:04 PST, Brady Eidson
sullivan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Beckinsale 2006-11-22 02:52:59 PST
The pages are not rendering correctly after using the back/forward buttons to navigate. This seems to be happening only after clicking a link that opens up a new window.

This bug behaves the same way as bug #11590, as the pages are loading in background, but don't seem to render anything visible. There are more ways to reproduce this, but this is the fastest and easiest I've encountered.

Steps to Reproduce:
1. Start from an existing page and go to
http://www.webdevelopersnotes.com/tutorials/adhtml/html_links_target_attribute_of_anchor_tag.php3

2. Click back then click forward (You will be on the test page again)
3. Open any of the test scripts and close the new window it creates
4. Click Back
Comment 1 Mark Rowe (bdash) 2006-11-22 03:01:21 PST
I can reproduce this with ToT.  I also see a crash due to a null Frame when clicking on the blank page:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x0000000c
0x010c9755 in WebCore::Frame::eventHandler (this=0x0) at WebCore/page/Frame.cpp:1453
1453        return &d->m_eventHandler;
(gdb) bt
#0  0x010c9755 in WebCore::Frame::eventHandler (this=0x0) at WebCore/page/Frame.cpp:1453
#1  0x0034c38b in -[WebHTMLView(WebDocumentInternalProtocols) elementAtPoint:allowShadowContent:] (self=0x181abfe0, _cmd=0x3b92c4, point={x = 454, y = 243}, allow=1 '\001') at WebKit/WebView/WebHTMLView.m:6144
#2  0x003427c6 in -[WebHTMLView _isSelectionEvent:] (self=0x181abfe0, _cmd=0x90a79648, event=0x18c79df0) at WebKit/WebView/WebHTMLView.m:2887
#3  0x00342a9d in -[WebHTMLView shouldDelayWindowOrderingForEvent:] (self=0x181abfe0, _cmd=0x90ac6138, event=0x18c79df0) at WebKit/WebView/WebHTMLView.m:2918
#4  0x9334b67c in -[NSWindow sendEvent:] ()
#5  0x0002338e in ?? ()
#6  0x9333e350 in -[NSApplication sendEvent:] ()
#7  0x00022f1e in ?? ()
#8  0x93268dfe in -[NSApplication run] ()
#9  0x9325cd2f in NSApplicationMain ()
#10 0x0005f7de in ?? ()
#11 0x0005f6f9 in ?? ()
Current language:  auto; currently c++
(gdb) 
Comment 2 Mark Rowe (bdash) 2006-11-22 03:31:08 PST
Local testing reveals that r17871, the fix for bug 11669, is the cause of this problem.
Comment 3 mitz 2006-11-24 02:19:05 PST
The buggy behavior results from navigating back/forward to a WebHTMLView that's already been -close'd. This problem has existed prior to r17871, but clearing the private data (in particular, the data source) made it more evident. What I've seen is that -[WebHTMLView close] is called from +[WebHistoryItem _closeObjectsInPendingPageCaches], but the WebFrame has its own WebHistoryItem which holds a pageCache which references the now-closed WebHTMLView. When you navigate back/forward, the WebHTMLView is pulled from there and reused despite having been closed.
Comment 4 mitz 2006-11-24 04:38:14 PST
The crash can be reproduced with any two cacheable pages, if the timing is correct (because of the 2.5-second pageCache release timer). You can use
defaults write com.apple.Safari WebKitLogLevel -string 00080000
to track the events.
1) Go to data:text/html,A
2) In the same window, go to data:text/html,B
3) Go back to A and quickly go forward to B
4) Wait 5 seconds (or until you see "releasing 2 items" it logging is enabled).
5) Go back to A and click anywhere on the page

In step 3, when you navigate back to A, its pageCache, referencing its WebHTMLView, is scheduled for release, but then when you navigate forward to B, a new pageCache is created for A, referencing the same WebHTMLView. 2.5 seconds later the original pageCache is released and the WebHTMLView is closed.

Currently a WebHistoryItem has no way of knowing of pageCaches that it has scheduled for release, but perhaps this can be changed by maintaining a set of "WebHistoryItems pending pageCache release" instead of the current set of "pageCaches pending release". Then this set could be checked against in -setHasPageCache:YES.

To see the effects of this bug before r17871, use an "A" page that has links or hover effects and notice that after step 5, the hover effects don't work and the cursor and Safari status bar don't change over links.
Comment 5 Chris Petersen 2006-11-24 22:35:27 PST
With r17878, I'm encountering this same issue as described when navigating (back/forward) between cached pages in my history.
Comment 6 Brady Eidson 2006-12-06 15:15:14 PST
I'll have this one figured out shortly
Comment 7 Brady Eidson 2006-12-06 15:31:24 PST
As a result of r17871, releasing a page cache now causes the WebHTMLView's datasource to be release and nil'd out.  I've confirmed that commenting out the lines in Darin's patch which do that fix this regression.
Comment 8 Brady Eidson 2006-12-06 16:04:10 PST
Created attachment 11759 [details]
Darin and I hashed this one out
Comment 9 John Sullivan 2006-12-06 16:48:56 PST
Comment on attachment 11759 [details]
Darin and I hashed this one out

Thank goodness.
Comment 10 Brady Eidson 2006-12-06 16:59:01 PST
Committed in r18049

Please verify!
Comment 11 mitz 2006-12-06 23:24:01 PST
(In reply to comment #10)
> Committed in r18049
> 
> Please verify!
> 

I was going to comment that "reopening" the WebHTMLView doesn't resume listening for WebPreferencesChangedNotification, so when you're back in the "A" page, changing the showsURLsInToolTips preference will have no effect. I made an app to test it, and while it's true, there's an even bigger problem with that preference, in that the first time _resetCachedWebPreferences is called, to pick up the initial value, [self _webView] is still nil, so the value is taken from the standard preferences.

I will file a separate bug on that.
Comment 12 mitz 2006-12-07 08:18:33 PST
(In reply to comment #11)

> I will file a separate bug on that.
> 

Bug 11775.