Bug 31145

Summary: Need notification of scrolling frame
Product: WebKit Reporter: Scott Violet <sky>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fishd, mitz, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Adds FrameLoaderClient::didChangeScrollOffset
fishd: review-
Revised patch none

Scott Violet
Reported 2009-11-04 16:45:15 PST
The notification will be used to know when history state is out of date.
Attachments
Adds FrameLoaderClient::didChangeScrollOffset (1.82 KB, patch)
2009-11-04 16:47 PST, Scott Violet 		fishd: review-
DetailsFormatted DiffDiff
Revised patch (1.91 KB, patch)
2009-11-05 07:56 PST, Scott Violet 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Scott Violet
Comment 1 2009-11-04 16:47:08 PST
Created attachment 42532 [details] Adds FrameLoaderClient::didChangeScrollOffset
Sam Weinig
Comment 2 2009-11-04 17:59:58 PST
Can you please explain what you mean by the "history state is out of date"?
Darin Fisher (:fishd, Google)
Comment 3 2009-11-04 23:21:46 PST
The frame's current scroll offset is a property of HistoryItem. However that field is only updated when HistoryController::saveDocumentAndScrollState() is called, which does not ordinarily happen when a frame scrolls. Chromium needs a way to know when the scroll offset for any frame has changed, so that it can schedule a call to saveDocumentAndScrollState(). Our objective is to use this as a signal to save session information for Chromium's session restore feature.
Darin Fisher (:fishd, Google)
Comment 4 2009-11-04 23:22:55 PST
Comment on attachment 42532 [details] Adds FrameLoaderClient::didChangeScrollOffset > Index: WebCore/ChangeLog .. > + Reviewed by NOBODY (OOPS!). > + > + Adds FrameLoaderClient::didChangeScrollOffset that is called when > + the frame scrolls. This will be used to know when history state > + needs to be updated. ^^^ fix the indentation please include a link to this bug. delete the line below: > + No new tests. LGTM otherwise
Scott Violet
Comment 5 2009-11-05 07:56:14 PST
Created attachment 42571 [details] Revised patch
Scott Violet
Comment 6 2009-11-05 07:57:08 PST
Sam, sorry for not being clear on why I want to add this. Darin, thanks for clarifying!
Dimitri Glazkov (Google)
Comment 7 2009-11-05 13:46:20 PST
Comment on attachment 42571 [details] Revised patch r=me per Darin's comments.
WebKit Commit Bot
Comment 8 2009-11-05 15:08:55 PST
Comment on attachment 42571 [details] Revised patch Clearing flags on attachment: 42571 Committed r50580: <http://trac.webkit.org/changeset/50580>
WebKit Commit Bot
Comment 9 2009-11-05 15:09:01 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.