Bug 56340 - Add WKViewSetScrollOffsetOnNextResize() to C API on Windows
Summary: Add WKViewSetScrollOffsetOnNextResize() to C API on Windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Jeff Miller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-14 15:39 PDT by Jeff Miller
Modified: 2011-03-14 16:53 PDT (History)
1 user (show)

See Also:


Attachments
Patch (4.46 KB, patch)
2011-03-14 15:49 PDT, Jeff Miller
no flags Details | Formatted Diff | Diff
Patch (4.53 KB, patch)
2011-03-14 16:43 PDT, Jeff Miller
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff Miller 2011-03-14 15:39:24 PDT
This is the Windows version of bug 53814.
Comment 1 Jeff Miller 2011-03-14 15:40:29 PDT
Unlike the Mac, on Windows we just need to be able to set the scroll offset to be applied the next time the view is sized, since we change the size separately.
Comment 2 Jeff Miller 2011-03-14 15:49:57 PDT
Created attachment 85733 [details]
Patch
Comment 3 Adam Roben (:aroben) 2011-03-14 16:02:39 PDT
Comment on attachment 85733 [details]
Patch

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

The "on next size" terminology is a little confusing. "on next resize" would be slightly clearer. Is there any downside to making the Windows API match the Mac API?

> Source/WebKit2/UIProcess/win/WebView.cpp:503
>      if (m_page && m_page->drawingArea())
> -        m_page->drawingArea()->setSize(IntSize(width, height), IntSize());
> +        m_page->drawingArea()->setSize(IntSize(width, height), m_nextSizeScrollOffset);
> +    
> +    m_nextSizeScrollOffset = IntSize();

Should we only clear out m_nextSizeScrollOffset when we actually call setSize?

> Source/WebKit2/UIProcess/win/WebView.cpp:805
> +    ASSERT(m_nextSizeScrollOffset.isZero());

I don't think we can assert this. A WebKit client app could call this API twice in a row without allowing any messages to be processed.
Comment 4 Jeff Miller 2011-03-14 16:36:10 PDT
(In reply to comment #3)
> (From update of attachment 85733 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=85733&action=review
> 
> The "on next size" terminology is a little confusing. "on next resize" would be slightly clearer. Is there any downside to making the Windows API match the Mac API?

I'll change it to use on next resize.  I didn't attempt to match the Mac API since the resizing is done inside Safari.
> 
> > Source/WebKit2/UIProcess/win/WebView.cpp:503
> >      if (m_page && m_page->drawingArea())
> > -        m_page->drawingArea()->setSize(IntSize(width, height), IntSize());
> > +        m_page->drawingArea()->setSize(IntSize(width, height), m_nextSizeScrollOffset);
> > +    
> > +    m_nextSizeScrollOffset = IntSize();
> 
> Should we only clear out m_nextSizeScrollOffset when we actually call setSize?

Sounds good.

> 
> > Source/WebKit2/UIProcess/win/WebView.cpp:805
> > +    ASSERT(m_nextSizeScrollOffset.isZero());
> 
> I don't think we can assert this. A WebKit client app could call this API twice in a row without allowing any messages to be processed.

I will remove the assert.
Comment 5 Jeff Miller 2011-03-14 16:43:27 PDT
Created attachment 85743 [details]
Patch
Comment 6 Jeff Miller 2011-03-14 16:53:05 PDT
Committed r81082: <http://trac.webkit.org/changeset/81082>