Summary: | WebFrame should have a method to determine its visual emptiness | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ian Henderson <ian> | ||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, beidson, joepeck, koivisto, webkit.review.bot, zalan | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac (Intel) | ||||||||||
OS: | OS X 10.6 | ||||||||||
Attachments: |
|
Description
Ian Henderson
2011-07-08 18:27:46 PDT
Created attachment 100197 [details]
proposed patch
Comment on attachment 100197 [details]
proposed patch
Do any of the other layout state flags need to be reset as well? E.g., m_hasPendingPostLayoutTasks seems like it might fall into the same bad state.
(In reply to comment #3) > (From update of attachment 100197 [details]) > Do any of the other layout state flags need to be reset as well? E.g., m_hasPendingPostLayoutTasks seems like it might fall into the same bad state. I'm not sure, maybe someone more familiar with WebCore layout can answer that. The issue here is that m_firstVisuallyNonEmptyLayoutCallbackPending is only set once, in FrameView's constructor. I guess the assumption was that page loads correspond to new FrameViews. This assumption is broken when a page is restored from the b/f cache. This fix looks rather scary to me. There is a ton of callbacks that are not made when a page comes back from b/f cache, and that even includes most basic loading callbacks. Sending didFirstVisuallyNonEmptyLayout alone could be confusing to clients. We can get the same result by asking the WebFrame if it's visually non-empty when the load commits. Updating bug title. Created attachment 101108 [details]
proposed patch
Comment on attachment 101108 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=101108&action=review > Source/WebKit/mac/WebView/WebFramePrivate.h:77 > - (BOOL)_firstLayoutDone; > +- (BOOL)_didFirstVisuallyNonEmptyLayout; I’d call this _firstVisuallyNonEmptyLayoutDone. I think a message named did… often implies that the sender did something and is notifying the receiver of that. Perhaps a better name for both would have been hasDone…. Comment on attachment 101108 [details]
proposed patch
Clearing the cq flag because of my naming suggestion.
Does this issue actually exist anymore? I believe we dispatch didFirstVisuallyNonEmptyLayout for cached pages too after http://trac.webkit.org/changeset/90900 (and the followup in 91027). GDB seems to agree. Comment on attachment 101108 [details]
proposed patch
This is no longer necessary.
Apparently this is necessary after all. I'll post a more up-to-date patch. Created attachment 101912 [details]
proposed patch
Comment on attachment 101912 [details] proposed patch Clearing flags on attachment: 101912 Committed r91727: <http://trac.webkit.org/changeset/91727> All reviewed patches have been landed. Closing bug. |