Bug 117172 - [CSS Exclusions][CSS Shapes] Split CSS Exclusions & Shapes compile & runtime flags
Summary: [CSS Exclusions][CSS Shapes] Split CSS Exclusions & Shapes compile & runtime ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Bear Travis
URL:
Keywords:
Depends on:
Blocks: 89256 117173
  Show dependency treegraph
 
Reported: 2013-06-03 16:04 PDT by Bear Travis
Modified: 2013-06-06 13:40 PDT (History)
5 users (show)

See Also:


Attachments
Initial patch (217.95 KB, patch)
2013-06-03 17:41 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Updated patch (201.09 KB, patch)
2013-06-04 10:32 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Updated patch (201.13 KB, patch)
2013-06-04 12:22 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
With GTK symbols (201.97 KB, patch)
2013-06-04 13:22 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Entabbing Windows Build Files (201.92 KB, patch)
2013-06-04 13:44 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Without .props modifications (190.85 KB, patch)
2013-06-04 15:06 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Updated Patch (202.02 KB, patch)
2013-06-04 16:00 PDT, Bear Travis
no flags Details | Formatted Diff | Diff
Without .props modifications (190.72 KB, patch)
2013-06-05 10:29 PDT, Bear Travis
achicu: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Updated patch (190.66 KB, patch)
2013-06-05 15:36 PDT, Bear Travis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bear Travis 2013-06-03 16:04:17 PDT
Fixup the compile flag to represent the two features
Comment 1 Bear Travis 2013-06-03 17:16:40 PDT
In addition to a new compile flag, there will need to be a new runtime flag (which the tests will need to use).
Comment 2 Bear Travis 2013-06-03 17:41:15 PDT
Created attachment 203636 [details]
Initial patch
Comment 3 Bear Travis 2013-06-04 10:32:12 PDT
Created attachment 203712 [details]
Updated patch
Comment 4 kov's GTK+ EWS bot 2013-06-04 11:29:12 PDT
Comment on attachment 203712 [details]
Updated patch

Attachment 203712 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/778027
Comment 5 Bear Travis 2013-06-04 12:22:48 PDT
Created attachment 203717 [details]
Updated patch
Comment 6 kov's GTK+ EWS bot 2013-06-04 12:58:16 PDT
Comment on attachment 203717 [details]
Updated patch

Attachment 203717 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/736274
Comment 7 Bear Travis 2013-06-04 13:22:03 PDT
Created attachment 203720 [details]
With GTK symbols
Comment 8 Bear Travis 2013-06-04 13:44:27 PDT
Created attachment 203723 [details]
Entabbing Windows Build Files
Comment 9 Bear Travis 2013-06-04 15:06:53 PDT
Created attachment 203728 [details]
Without .props modifications
Comment 10 Bear Travis 2013-06-04 16:00:15 PDT
Created attachment 203735 [details]
Updated Patch
Comment 11 Bear Travis 2013-06-05 10:29:05 PDT
Created attachment 203865 [details]
Without .props modifications
Comment 12 Alexandru Chiculita 2013-06-05 14:39:45 PDT
Comment on attachment 203865 [details]
Without .props modifications

Looks good.
Comment 13 WebKit Commit Bot 2013-06-05 14:54:12 PDT
Comment on attachment 203865 [details]
Without .props modifications

Rejecting attachment 203865 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 203865, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
inside-with-region-borders.html
patching file LayoutTests/fast/regions/shape-inside/shape-inside-with-region-padding.html
patching file ChangeLog
patching file WebKitLibraries/ChangeLog
patching file WebKitLibraries/win/tools32/vsprops/FeatureDefines.vsprops
patching file WebKitLibraries/win/tools32/vsprops/FeatureDefinesCairo.vsprops

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Alexandru Chiculita']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/786019
Comment 14 Bear Travis 2013-06-05 15:36:46 PDT
Created attachment 203890 [details]
Updated patch
Comment 15 WebKit Commit Bot 2013-06-05 16:06:00 PDT
Comment on attachment 203890 [details]
Updated patch

Clearing flags on attachment: 203890

Committed r151247: <http://trac.webkit.org/changeset/151247>
Comment 16 WebKit Commit Bot 2013-06-05 16:06:03 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 Hugo Parente Lima 2013-06-06 12:17:32 PDT
The build fails if CSS_EXCLUSION is ON but CSS_SHAPES is OFF, I'm not aware of CSS shapes/exclusion specs, so this build fail makes sense? I mean if CSS_EXCLUSION is ON CSS_SHAPES *must* be on too?

The fail happen at:

Source/WebCore/rendering/LayoutState.h:129:5: error: 'ExclusionShapeInsideInfo' does not name a type

because the forward declaration of ExclusionShapeInsideInfo is under CSS_SHAPES but there is a variable of this type (m_exclusionShapeInsideInfo) declared under CSS_EXCLUSIONS.
Comment 18 Bear Travis 2013-06-06 13:40:05 PDT
(In reply to comment #17)
> The build fails if CSS_EXCLUSION is ON but CSS_SHAPES is OFF, I'm not aware of CSS shapes/exclusion specs, so this build fail makes sense? I mean if CSS_EXCLUSION is ON CSS_SHAPES *must* be on too?
> 
> The fail happen at:
> 
> Source/WebCore/rendering/LayoutState.h:129:5: error: 'ExclusionShapeInsideInfo' does not name a type
> 
> because the forward declaration of ExclusionShapeInsideInfo is under CSS_SHAPES but there is a variable of this type (m_exclusionShapeInsideInfo) declared under CSS_EXCLUSIONS.

Good catch. The features should be separable, and in this case m_exclusionShapeInsideInfo should be guarded by CSS_SHAPES, not CSS_EXCLUSIONS. I have filed bug 117318 to track this.