Bug 61694

Summary: WebKit2: Enable serializing of data types needed for cross-process accelerated compositing
Product: WebKit Reporter: Noam Rosenthal <noam>
Component: WebKit wxAssignee: Noam Rosenthal <noam>
Status: RESOLVED FIXED    
Severity: Enhancement CC: abecsi, aroben, commit-queue, hausmann, igor.oliveira, noam, ostap73, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 62192    
Bug Blocks: 56935    
Attachments:
Description Flags
Patch 1: enable Length/TransformationMatrix via SimpleArgumentCoder
none
Patch 2: Serialize TimingFunction
none
Patch 3: ArgumentCoder for WebCore::Animation
commit-queue: commit-queue-
Patch 4: TransformOperation[s]
none
Patch 1: enable Length/TransformationMatrix via SimpleArgumentCoder
commit-queue: commit-queue-
Patch 2: Serialize TimingFunction
commit-queue: commit-queue-
Patch 3: ArgumentCoder for WebCore::Animation
none
Patch 1: enable Length/TransformationMatrix via SimpleArgumentCoder
none
Patch 2: Serialize TimingFunction
none
Patch for landing
none
Patch 3: ArgumentCoder for WebCore::Animation
none
Patch 4: TransformOperation[s]
none
Patch
none
Patch 4: TransformOperation[s]
webkit.review.bot: commit-queue-
Patch
none
Patch 4: TransformOperation[s]
none
Patch
none
Last patch: KeyframeValueList coder none

Description Noam Rosenthal 2011-05-28 15:00:11 PDT
This is the first part of upstreaming the cross-process accelerated-compositing implementation we use with Qt.
To enable accelerated compositing on the UI process, we need to serialize the following:
* TransformationMatrix
* TimingFunction
* TransformOperation[s]
* Animation
* The GraphicsLayer data

As a first measure, we'll enable ArgumentCoders for relevant existing data types.
Comment 1 Noam Rosenthal 2011-05-29 09:13:17 PDT
Created attachment 95287 [details]
Patch 1: enable Length/TransformationMatrix via SimpleArgumentCoder
Comment 2 Noam Rosenthal 2011-05-29 11:25:47 PDT
Created attachment 95289 [details]
Patch 2: Serialize TimingFunction
Comment 3 Noam Rosenthal 2011-05-29 15:25:10 PDT
Created attachment 95304 [details]
Patch 3: ArgumentCoder for WebCore::Animation
Comment 4 WebKit Commit Bot 2011-05-29 16:21:00 PDT
Comment on attachment 95304 [details]
Patch 3: ArgumentCoder for WebCore::Animation

Attachment 95304 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8747583
Comment 5 Noam Rosenthal 2011-05-29 16:26:36 PDT
(In reply to comment #4)
> (From update of attachment 95304 [details])
> Attachment 95304 [details] did not pass mac-ews (mac):
> Output: http://queues.webkit.org/results/8747583

This relies on the other patches to compile. It should still be reviewable.
Comment 6 Noam Rosenthal 2011-05-29 21:03:12 PDT
Created attachment 95320 [details]
Patch 4: TransformOperation[s]
Comment 7 Adam Roben (:aroben) 2011-05-30 08:31:31 PDT
Comment on attachment 95287 [details]
Patch 1: enable Length/TransformationMatrix via SimpleArgumentCoder

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

> Source/WebKit2/Scripts/webkit2/messages.py:447
> +        'WebCore::Length': '<WebCore/Length.h>',

I thought the scripts knew how to infer this. My understanding was that headers_for_type only needed to be modified if the header did not match the namespace/class name.
Comment 8 Noam Rosenthal 2011-05-30 09:30:14 PDT
(In reply to comment #7)
> (From update of attachment 95287 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=95287&action=review
> 
> > Source/WebKit2/Scripts/webkit2/messages.py:447
> > +        'WebCore::Length': '<WebCore/Length.h>',
> 
> I thought the scripts knew how to infer this. My understanding was that headers_for_type only needed to be modified if the header did not match the namespace/class name.

This is needed, otherwise the headers don't get copied to the include/WebCore directory.
Comment 9 Adam Roben (:aroben) 2011-05-30 09:32:06 PDT
Comment on attachment 95287 [details]
Patch 1: enable Length/TransformationMatrix via SimpleArgumentCoder

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

>>> Source/WebKit2/Scripts/webkit2/messages.py:447
>>> +        'WebCore::Length': '<WebCore/Length.h>',
>> 
>> I thought the scripts knew how to infer this. My understanding was that headers_for_type only needed to be modified if the header did not match the namespace/class name.
> 
> This is needed, otherwise the headers don't get copied to the include/WebCore directory.

How does this script affect what headers get copied? All it's supposed to do is write generated source files.
Comment 10 Noam Rosenthal 2011-05-30 09:43:01 PDT
> > This is needed, otherwise the headers don't get copied to the include/WebCore directory.
> 
> How does this script affect what headers get copied? All it's supposed to do is write generated source files.

OK, It's needed in the script but it can be added to a different part of the script (not to special_cases). I'll amend before committing.
Comment 11 Noam Rosenthal 2011-05-30 09:52:04 PDT
> > This is needed, otherwise the headers don't get copied to the include/WebCore directory.
> 
> How does this script affect what headers get copied? All it's supposed to do is write generated source files.

OK, It's needed in the script but it can be added to a different part of the script (not to special_cases). I'll amend before committing.
Comment 12 Noam Rosenthal 2011-05-30 09:53:38 PDT
Created attachment 95349 [details]
Patch 1: enable Length/TransformationMatrix via SimpleArgumentCoder
Comment 13 Noam Rosenthal 2011-05-30 10:10:41 PDT
Created attachment 95350 [details]
Patch 2: Serialize TimingFunction
Comment 14 Noam Rosenthal 2011-05-30 10:39:41 PDT
Created attachment 95354 [details]
Patch 3: ArgumentCoder for WebCore::Animation
Comment 15 WebKit Commit Bot 2011-05-30 12:06:01 PDT
Comment on attachment 95350 [details]
Patch 2: Serialize TimingFunction

Rejecting attachment 95350 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 2

Last 500 characters of output:
tching file Source/WebKit2/Scripts/webkit2/messages.py
Hunk #1 FAILED at 258.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit2/Scripts/webkit2/messages.py.rej
patching file Source/WebKit2/Shared/WebCoreArgumentCoders.h
Hunk #1 FAILED at 48.
Hunk #2 succeeded at 481 (offset -5 lines).
1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit2/Shared/WebCoreArgumentCoders.h.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/8747754
Comment 16 WebKit Commit Bot 2011-05-30 12:30:29 PDT
Comment on attachment 95349 [details]
Patch 1: enable Length/TransformationMatrix via SimpleArgumentCoder

Rejecting attachment 95349 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'land-a..." exit_code: 1

Last 500 characters of output:
pe=xml
Processing 1 patch from 1 bug.
Cleaning working directory
Updating working directory
Processing patch 95349 from bug 61694.
Simon Haussman found in /mnt/git/webkit-commit-queue/Source/WebKit2/ChangeLog does not appear to be a valid reviewer according to committers.py.
ERROR: /mnt/git/webkit-commit-queue/Source/WebKit2/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Updating OpenSource
Current branch master is up to date.

Full output: http://queues.webkit.org/results/8751503
Comment 17 Noam Rosenthal 2011-05-30 12:37:48 PDT
Created attachment 95360 [details]
Patch 1: enable Length/TransformationMatrix via SimpleArgumentCoder
Comment 18 Noam Rosenthal 2011-05-30 13:32:40 PDT
Created attachment 95366 [details]
Patch 2: Serialize TimingFunction
Comment 19 WebKit Commit Bot 2011-05-30 13:58:43 PDT
Comment on attachment 95366 [details]
Patch 2: Serialize TimingFunction

Rejecting attachment 95366 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'apply-..." exit_code: 2

Last 500 characters of output:
tching file Source/WebKit2/Scripts/webkit2/messages.py
Hunk #1 FAILED at 258.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit2/Scripts/webkit2/messages.py.rej
patching file Source/WebKit2/Shared/WebCoreArgumentCoders.h
Hunk #1 FAILED at 48.
Hunk #2 succeeded at 481 (offset -5 lines).
1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit2/Shared/WebCoreArgumentCoders.h.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/8753229
Comment 20 WebKit Commit Bot 2011-05-30 15:03:08 PDT
Comment on attachment 95360 [details]
Patch 1: enable Length/TransformationMatrix via SimpleArgumentCoder

Clearing flags on attachment: 95360

Committed r87699: <http://trac.webkit.org/changeset/87699>
Comment 21 WebKit Review Bot 2011-05-30 15:09:17 PDT
Attachment 95366 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebKit2/Shared/WebCoreArgumentCoders.h:499:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 Noam Rosenthal 2011-05-30 15:41:03 PDT
Created attachment 95377 [details]
Patch for landing
Comment 23 WebKit Commit Bot 2011-05-30 16:44:11 PDT
Comment on attachment 95377 [details]
Patch for landing

Clearing flags on attachment: 95377

Committed r87701: <http://trac.webkit.org/changeset/87701>
Comment 24 Noam Rosenthal 2011-05-30 16:51:46 PDT
Created attachment 95381 [details]
Patch 3: ArgumentCoder for WebCore::Animation
Comment 25 WebKit Commit Bot 2011-05-30 19:22:01 PDT
Comment on attachment 95381 [details]
Patch 3: ArgumentCoder for WebCore::Animation

Clearing flags on attachment: 95381

Committed r87707: <http://trac.webkit.org/changeset/87707>
Comment 26 Noam Rosenthal 2011-06-06 22:40:21 PDT
Created attachment 96206 [details]
Patch 4: TransformOperation[s]
Comment 27 WebKit Review Bot 2011-06-06 22:42:01 PDT
Attachment 96206 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/Shared/WebCoreArgumentCoders.h:49:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/Shared/WebCoreArgumentCoders.h:62:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 28 Noam Rosenthal 2011-06-06 22:46:37 PDT
Created attachment 96209 [details]
Patch
Comment 29 Noam Rosenthal 2011-06-06 22:52:49 PDT
Created attachment 96210 [details]
Patch 4: TransformOperation[s]
Comment 30 WebKit Review Bot 2011-06-06 23:20:27 PDT
Comment on attachment 96210 [details]
Patch 4: TransformOperation[s]

Clearing flags on attachment: 96210

Committed r88222: <http://trac.webkit.org/changeset/88222>
Comment 31 Noam Rosenthal 2011-06-07 09:36:24 PDT
Not sure why this (In reply to comment #30)
> (From update of attachment 96210 [details])
> Clearing flags on attachment: 96210
> 
> Committed r88222: <http://trac.webkit.org/changeset/88222>

Not sure why this broke the Mac build; It builds fine for me. It seems like I'm not putting the transform operation headers in the right place in the python script, but I'm not sure where they're supposed to go.
Comment 32 WebKit Review Bot 2011-06-07 22:10:44 PDT
Comment on attachment 96210 [details]
Patch 4: TransformOperation[s]

Attachment 96210 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8810032
Comment 33 Noam Rosenthal 2011-06-08 10:47:08 PDT
Created attachment 96440 [details]
Patch
Comment 34 Noam Rosenthal 2011-06-08 12:17:21 PDT
Created attachment 96457 [details]
Patch 4: TransformOperation[s]
Comment 35 Noam Rosenthal 2011-06-08 12:19:23 PDT
(In reply to comment #34)
> Created an attachment (id=96457) [details]
> Patch 4: TransformOperation[s]

This includes some build fixes for Mac.
Comment 36 WebKit Review Bot 2011-06-08 13:07:22 PDT
Comment on attachment 96457 [details]
Patch 4: TransformOperation[s]

Clearing flags on attachment: 96457

Committed r88378: <http://trac.webkit.org/changeset/88378>
Comment 37 WebKit Review Bot 2011-06-08 13:07:29 PDT
All reviewed patches have been landed.  Closing bug.
Comment 38 Noam Rosenthal 2011-06-08 13:15:17 PDT
Not done yet... :)
Comment 39 Noam Rosenthal 2011-06-08 13:19:34 PDT
Created attachment 96469 [details]
Patch
Comment 40 WebKit Review Bot 2011-06-08 14:53:54 PDT
Comment on attachment 96469 [details]
Patch

Clearing flags on attachment: 96469

Committed r88392: <http://trac.webkit.org/changeset/88392>
Comment 41 WebKit Review Bot 2011-06-08 14:54:01 PDT
All reviewed patches have been landed.  Closing bug.
Comment 42 Noam Rosenthal 2011-06-08 18:06:58 PDT
Created attachment 96522 [details]
Last patch: KeyframeValueList coder
Comment 43 Noam Rosenthal 2011-06-08 18:19:57 PDT
Opening for last patch.
Comment 44 Simon Hausmann 2011-10-25 05:54:55 PDT
Comment on attachment 96522 [details]
Last patch: KeyframeValueList coder

r=me
Comment 45 Igor Trindade Oliveira 2011-11-18 10:46:54 PST
Committed r100789: http://trac.webkit.org/changeset/100789


(In reply to comment #44)
> (From update of attachment 96522 [details])
> r=me
Comment 46 Noam Rosenthal 2011-12-06 17:03:02 PST
Comment on attachment 96522 [details]
Last patch: KeyframeValueList coder

Committed.
Comment 47 Noam Rosenthal 2011-12-06 17:03:21 PST
All patches have landed