Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2563753003:
Moved pointer-events to be generated in ComputedStyleBase (Closed)
DescriptionMove the inherited property pointer-events and its enum, EPointerEvents,
to be generated in ComputedStyleBase. Also removed the explicit value
setters for EPointerEvents, and the now empty compareEqualIndependent()
method.
BUG=628043
Committed: https://crrev.com/604db4a0dc2613b803899d22ccfe76ed262c6207
Cr-Commit-Position: refs/heads/master@{#438457}
Patch Set 1 #Patch Set 2 : Corrected initial keyword to be auto. #
Total comments: 6
Patch Set 3 : Moved logic in compareEqualNonIndependent() into operator==() and removed compareEqualNonIndependen… #Patch Set 4 : Fixed compilation error in nonInheritedEqual. #Patch Set 5 : Rebase #
Messages
Total messages: 40 (30 generated)
The CQ bit was checked by napper@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
The CQ bit was checked by napper@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Move the inherited property pointer-events and its enum, EPointerEvents, to be generated in ComputedStyleBase. Also removed the explicit value setters for EPointerEvents. BUG=628043 ========== to ========== Move the inherited property pointer-events and its enum, EPointerEvents, to be generated in ComputedStyleBase. Also removed the explicit value setters for EPointerEvents, and the now empty compareEqualIndependent() method. BUG=628043 ==========
napper@chromium.org changed reviewers: + sashab@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by napper@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Once you've done feedback send to alancutter@ for review https://codereview.chromium.org/2563753003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/ComputedStyle.h (left): https://codereview.chromium.org/2563753003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:211: // CSSProperties.in. Move this comment to the generated compareEqualIndependent() method https://codereview.chromium.org/2563753003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:337: // to the existing flags to take advantage of packing as much as possible. Move this comment somewhere... Maybe into make_computed_style_base.py https://codereview.chromium.org/2563753003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/ComputedStyle.h (right): https://codereview.chromium.org/2563753003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:201: return compareEqualNonIndependent(other); Move all checks into here, delete compareEqualNonIndependent() function too
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by napper@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
napper@chromium.org changed reviewers: + alancutter@chromium.org
https://codereview.chromium.org/2563753003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/ComputedStyle.h (left): https://codereview.chromium.org/2563753003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:211: // CSSProperties.in. On 2016/12/12 at 01:52:56, sashab wrote: > Move this comment to the generated compareEqualIndependent() method Not sure what you mean here. The compareEqualIndependent() method doesn't exist anymore. https://codereview.chromium.org/2563753003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:337: // to the existing flags to take advantage of packing as much as possible. On 2016/12/12 at 01:52:56, sashab wrote: > Move this comment somewhere... Maybe into make_computed_style_base.py As discussed offline, I have reviewed the relevance of this comment. 1. The first 2 sentences are already documented in make_computed_style_base.py 2. The remainder is not required now there are no independent but not generated properties. https://codereview.chromium.org/2563753003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/style/ComputedStyle.h (right): https://codereview.chromium.org/2563753003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/style/ComputedStyle.h:201: return compareEqualNonIndependent(other); On 2016/12/12 at 01:52:56, sashab wrote: > Move all checks into here, delete compareEqualNonIndependent() function too Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by napper@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by napper@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for third_party/WebKit/Source/core/style/ComputedStyle.h:
While running git apply --index -p1;
error: patch failed: third_party/WebKit/Source/core/style/ComputedStyle.h:365
error: third_party/WebKit/Source/core/style/ComputedStyle.h: patch does not
apply
Patch: third_party/WebKit/Source/core/style/ComputedStyle.h
Index: third_party/WebKit/Source/core/style/ComputedStyle.h
diff --git a/third_party/WebKit/Source/core/style/ComputedStyle.h
b/third_party/WebKit/Source/core/style/ComputedStyle.h
index
225dcbb2fea09328939f16314bf69bca5c2337fd..88ec2bb5793fb50e5967512ae880c7cbc4cadeab
100644
--- a/third_party/WebKit/Source/core/style/ComputedStyle.h
+++ b/third_party/WebKit/Source/core/style/ComputedStyle.h
@@ -198,24 +198,6 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
// inherit
struct InheritedData {
bool operator==(const InheritedData& other) const {
- return compareEqualIndependent(other) &&
- compareEqualNonIndependent(other);
- }
-
- bool operator!=(const InheritedData& other) const {
- return !(*this == other);
- }
-
- inline bool compareEqualIndependent(const InheritedData& other) const {
- // These must match the properties tagged 'independent' in
- // CSSProperties.in.
- // TODO(napper): Remove this once all independent properties are
- // generated and replace with a private function used only in
- // stylePropagationDiff().
- return (m_pointerEvents == other.m_pointerEvents);
- }
-
- inline bool compareEqualNonIndependent(const InheritedData& other) const {
return (m_listStyleType == other.m_listStyleType) &&
(m_textAlign == other.m_textAlign) &&
(m_hasSimpleUnderline == other.m_hasSimpleUnderline) &&
@@ -228,6 +210,10 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
(m_writingMode == other.m_writingMode);
}
+ bool operator!=(const InheritedData& other) const {
+ return !(*this == other);
+ }
+
unsigned m_listStyleType : 7; // EListStyleType
unsigned m_textAlign : 4; // ETextAlign
unsigned m_hasSimpleUnderline : 1; // True if 'underline solid' is the
only
@@ -241,7 +227,6 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
// non CSS2 inherited
unsigned m_rtlOrdering : 1; // EOrder
unsigned m_printColorAdjust : 1; // PrintColorAdjust
- unsigned m_pointerEvents : 4; // EPointerEvents
unsigned m_insideLink : 2; // EInsideLink
// CSS Text Layout Module Level 3: Vertical writing support
@@ -327,25 +312,6 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
mutable unsigned m_hasRemUnits : 1;
- // For each independent inherited property, store a 1 if the stored
- // value was inherited from its parent, or 0 if it is explicitly set on
- // this element.
- // Eventually, all properties will have a bit in here to store whether
- // they were inherited from their parent or not.
- // Although two ComputedStyles are equal if their nonInheritedData is
- // equal regardless of the isInherited flags, this struct is stored next
- // to the existing flags to take advantage of packing as much as possible.
- // TODO(sashab): Move these flags closer to inheritedData so that it's
- // clear which inherited properties have a flag stored and which don't.
- // Keep this list of fields in sync with:
- // - setBitDefaults()
- // - The ComputedStyle setter, which must take an extra boolean parameter
- // and set this - propagateIndependentInheritedProperties() in
- // ComputedStyle.cpp
- // - The compareEqual() methods in the corresponding class
- // InheritedFlags
- unsigned m_isPointerEventsInherited : 1;
-
// If you add more style bits here, you will also need to update
// ComputedStyle::copyNonInheritedFromCached() 68 bits
} m_nonInheritedData;
@@ -365,8 +331,6 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
static_cast<unsigned>(initialBoxDirection());
m_inheritedData.m_printColorAdjust =
static_cast<unsigned>(initialPrintColorAdjust());
- m_inheritedData.m_pointerEvents =
- static_cast<unsigned>(initialPointerEvents());
m_inheritedData.m_insideLink = NotInsideLink;
m_inheritedData.m_writingMode = initialWritingMode();
@@ -397,9 +361,6 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
m_nonInheritedData.m_affectedByDrag = false;
m_nonInheritedData.m_isLink = false;
m_nonInheritedData.m_hasRemUnits = false;
-
- // All independently inherited properties default to being inherited.
- m_nonInheritedData.m_isPointerEventsInherited = true;
}
private:
@@ -2132,18 +2093,6 @@ class CORE_EXPORT ComputedStyle : public
ComputedStyleBase,
SET_VAR(m_rareInheritedData, overflowWrap, b);
}
- // pointer-events
- static EPointerEvents initialPointerEvents() { return EPointerEvents::Auto; }
- EPointerEvents pointerEvents() const {
- return static_cast<EPointerEvents>(m_inheritedData.m_pointerEvents);
- }
- void setPointerEvents(EPointerEvents p) {
- m_inheritedData.m_pointerEvents = static_cast<unsigned>(p);
- }
- void setPointerEventsIsInherited(bool isInherited) {
- m_nonInheritedData.m_isPointerEventsInherited = isInherited;
- }
-
// quotes
static QuotesData* initialQuotes() { return 0; }
QuotesData* quotes() const { return m_rareInheritedData->quotes.get(); }
The CQ bit was checked by napper@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sashab@chromium.org Link to the patchset: https://codereview.chromium.org/2563753003/#ps80001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481694242235990,
"parent_rev": "77da6759869d460e035b30862fca141522c95e6d", "commit_rev":
"b1612be29c30b09936f1b2f3b8bee5a4232e5dc7"}
Message was sent while issue was closed.
Description was changed from ========== Move the inherited property pointer-events and its enum, EPointerEvents, to be generated in ComputedStyleBase. Also removed the explicit value setters for EPointerEvents, and the now empty compareEqualIndependent() method. BUG=628043 ========== to ========== Move the inherited property pointer-events and its enum, EPointerEvents, to be generated in ComputedStyleBase. Also removed the explicit value setters for EPointerEvents, and the now empty compareEqualIndependent() method. BUG=628043 Review-Url: https://codereview.chromium.org/2563753003 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Move the inherited property pointer-events and its enum, EPointerEvents, to be generated in ComputedStyleBase. Also removed the explicit value setters for EPointerEvents, and the now empty compareEqualIndependent() method. BUG=628043 Review-Url: https://codereview.chromium.org/2563753003 ========== to ========== Move the inherited property pointer-events and its enum, EPointerEvents, to be generated in ComputedStyleBase. Also removed the explicit value setters for EPointerEvents, and the now empty compareEqualIndependent() method. BUG=628043 Committed: https://crrev.com/604db4a0dc2613b803899d22ccfe76ed262c6207 Cr-Commit-Position: refs/heads/master@{#438457} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/604db4a0dc2613b803899d22ccfe76ed262c6207 Cr-Commit-Position: refs/heads/master@{#438457} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
