Bug 114469

Summary: Add contentAnchor to WKView
Product: WebKit Reporter: Gavin Barraclough <barraclough>
Component: WebKit2Assignee: Gavin Barraclough <barraclough>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, cmarcelo, commit-queue, gyuyoung.kim, menard, rakuco, rniwa, simon.fraser, thorton, webkit-ews
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
simon.fraser: review-, eflews.bot: commit-queue-
Reset layer origin on next setFrameSize with updates enabled
simon.fraser: review+, eflews.bot: commit-queue-
use inline functions, more qt/efl build fixes none

Description Gavin Barraclough 2013-04-11 15:37:52 PDT
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.
Comment 1 Gavin Barraclough 2013-04-11 16:13:25 PDT
Created attachment 197693 [details]
Patch
Comment 2 WebKit Commit Bot 2013-04-11 16:15:19 PDT
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.
Comment 3 EFL EWS Bot 2013-04-11 16:16:51 PDT
Comment on attachment 197693 [details]
Patch

Attachment 197693 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/65015
Comment 4 Early Warning System Bot 2013-04-11 16:28:54 PDT
Comment on attachment 197693 [details]
Patch

Attachment 197693 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-queues.appspot.com/results/101015
Comment 5 Simon Fraser (smfr) 2013-04-11 18:05:12 PDT
Comment on attachment 197693 [details]
Patch

Gavin and I discussed this and he's going to make a few more changes.
Comment 6 Gavin Barraclough 2013-04-12 15:21:21 PDT
Created attachment 197895 [details]
Reset layer origin on next setFrameSize with updates enabled
Comment 7 WebKit Commit Bot 2013-04-12 15:23:41 PDT
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.
Comment 8 EFL EWS Bot 2013-04-12 15:29:32 PDT
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
Comment 9 Simon Fraser (smfr) 2013-04-12 15:38:27 PDT
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.
Comment 10 Early Warning System Bot 2013-04-12 15:51:25 PDT
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
Comment 11 Gavin Barraclough 2013-04-12 15:56:56 PDT
Created attachment 197899 [details]
use inline functions, more qt/efl build fixes
Comment 12 WebKit Commit Bot 2013-04-12 15:59:20 PDT
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.
Comment 13 Gavin Barraclough 2013-04-12 16:26:17 PDT
Fixed in http://trac.webkit.org/changeset/148311
Comment 14 Darin Adler 2013-04-12 16:56:04 PDT
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.
Comment 15 Ryosuke Niwa 2013-04-12 17:21:09 PDT
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) {
~~~~~~~ ^
Comment 16 Gavin Barraclough 2013-04-12 18:38:26 PDT
build fixes:
Changeset [148322] by barraclough@apple.com
Changeset [148324] by barraclough@apple.com
Changeset [148330] by jberlin@webkit.org