Bug 114469 – Add contentAnchor to WKView

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Gavin Barraclough]

Currently if you call setFrameSize: while frame size updates are disabled, or if the geometry update times out, then the previously rendered page content will be rendered anchored to the top left corner of the frame.

This is appropriate if the frame is being resized from the bottom right corner.  In order to achieve a more desirable appearance if the frame is being resized from another corner we should allow the corner at which the content anchors to be specified.

[Gavin Barraclough]

Created attachment 197693 [details]
Patch

[WebKit Commit Bot]

Attachment 197693 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/mac/WKView.mm', u'Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h', u'Source/WebKit2/UIProcess/DrawingAreaProxy.cpp', u'Source/WebKit2/UIProcess/DrawingAreaProxy.h', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.messages.in', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm']" exit_code: 1
Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h:28:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EFL EWS Bot]

Comment on attachment 197693 [details]
Patch

Attachment 197693 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/65015

[Early Warning System Bot]

Comment on attachment 197693 [details]
Patch

Attachment 197693 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-queues.appspot.com/results/101015

[Simon Fraser (smfr)]

Comment on attachment 197693 [details]
Patch

Gavin and I discussed this and he's going to make a few more changes.

[Gavin Barraclough]

Created attachment 197895 [details]
Reset layer origin on next setFrameSize with updates enabled

[WebKit Commit Bot]

Attachment 197895 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/mac/WKView.mm', u'Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h', u'Source/WebKit2/UIProcess/API/qt/raw/qrawwebview.cpp', u'Source/WebKit2/UIProcess/DrawingAreaProxy.cpp', u'Source/WebKit2/UIProcess/DrawingAreaProxy.h', u'Source/WebKit2/UIProcess/PageViewportController.cpp', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.messages.in', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm']" exit_code: 1
Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h:28:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EFL EWS Bot]

Comment on attachment 197895 [details]
Reset layer origin on next setFrameSize with updates enabled

Attachment 197895 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/48266

[Simon Fraser (smfr)]

Comment on attachment 197895 [details]
Reset layer origin on next setFrameSize with updates enabled

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

> Source/WebKit2/UIProcess/API/mac/WKView.mm:98
> +#define WKContentAnchorRight(x) (!!((x) & WKContentAnchorTopRight))
> +#define WKContentAnchorBottom(x) (!!((x) & WKContentAnchorBottomLeft))

Wouldn't inline functions be preferable in C++?

bool WKContentAnchorRight(unsigned x) { return x & WKContentAnchorTopRight; }

But WKContentAnchor* are not bit masks, so this is vulnerable to someone re-ordering WKContentAnchor flags. I think an explicit  x == WKContentAnchorTopRight || x == WKContentAnchorBottomRight would be better.

[Early Warning System Bot]

Comment on attachment 197895 [details]
Reset layer origin on next setFrameSize with updates enabled

Attachment 197895 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-queues.appspot.com/results/24012

[Gavin Barraclough]

Created attachment 197899 [details]
use inline functions, more qt/efl build fixes

[WebKit Commit Bot]

Attachment 197899 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/mac/WKView.mm', u'Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h', u'Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp', u'Source/WebKit2/UIProcess/API/qt/raw/qrawwebview.cpp', u'Source/WebKit2/UIProcess/DrawingAreaProxy.cpp', u'Source/WebKit2/UIProcess/DrawingAreaProxy.h', u'Source/WebKit2/UIProcess/PageViewportController.cpp', u'Source/WebKit2/UIProcess/efl/WebView.cpp', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.h', u'Source/WebKit2/UIProcess/mac/TiledCoreAnimationDrawingAreaProxy.mm', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/DrawingArea.messages.in', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.h', u'Source/WebKit2/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm']" exit_code: 1
Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h:28:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gavin Barraclough]

Fixed in http://trac.webkit.org/changeset/148311

[Darin Adler]

Comment on attachment 197895 [details]
Reset layer origin on next setFrameSize with updates enabled

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

>> Source/WebKit2/UIProcess/API/mac/WKView.mm:98
>> +#define WKContentAnchorBottom(x) (!!((x) & WKContentAnchorBottomLeft))
> 
> Wouldn't inline functions be preferable in C++?
> 
> bool WKContentAnchorRight(unsigned x) { return x & WKContentAnchorTopRight; }
> 
> But WKContentAnchor* are not bit masks, so this is vulnerable to someone re-ordering WKContentAnchor flags. I think an explicit  x == WKContentAnchorTopRight || x == WKContentAnchorBottomRight would be better.

You can use COMPILE_ASSERT to make a safe version that still uses & for efficiency.

[Ryosuke Niwa]

There's another build failure from this patch:

http://build.webkit.org/builders/Apple%20Lion%20Debug%20%28Build%29/builds/15014

In file included from /Volumes/Data/slave/lion-debug/build/Source/WebKit2/UIProcess/API/mac/PDFViewController.mm:31:
/Volumes/Data/slave/lion-debug/build/Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h:28:29: error: C++ requires a type specifier for all declarations
typedef NS_ENUM(NSUInteger, WKContentAnchor) {
                            ^~~~~~~~~~~~~~~
/Volumes/Data/slave/lion-debug/build/Source/WebKit2/UIProcess/API/mac/WKViewPrivate.h:28:9: error: C++ requires a type specifier for all declarations
typedef NS_ENUM(NSUInteger, WKContentAnchor) {
~~~~~~~ ^

[Gavin Barraclough]

build fixes:
Changeset [148322] by [e-mail]
Changeset [148324] by [e-mail]
Changeset [148330] by [e-mail]