Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(159)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 912383003: ASSERTION FAILED: !m_parsedCalculation in CSSPropertyParser::parseCalculation (Closed)

Created:
5 years, 10 months ago by rwlbuis
Modified:
5 years, 10 months ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, dglazkov+blink, ed+blinkwatch_opera.com
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

ASSERTION FAILED: !m_parsedCalculation in CSSPropertyParser::parseCalculation After calling validUnit a CSSPrimitiveValue is supposed to be created. In this case we do two validUnit calls in succession without creating a CSSPrimitiveValue and clearing m_parsedCalculation, hitting the ASSERT in parseCalculation on the second validUnit call. So single out m_parsedCalculation and test it to be non negative, avoiding the second validUnit call. BUG=457649 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190493

Patch Set 1 #

Total comments: 2

Patch Set 2 : Adress reviewing comments #

Patch Set 3 : Remove unrelated change #

Total comments: 1

Patch Set 4 : Patch for landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -0 lines) Patch
A LayoutTests/fast/css/calc-in-text-shadow-property.html View 1 1 chunk +10 lines, -0 lines 0 comments Download
A + LayoutTests/fast/css/calc-in-text-shadow-property-expected.html View 1 0 chunks +-1 lines, --1 lines 0 comments Download
M Source/core/css/parser/CSSPropertyParser.cpp View 1 2 3 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 27 (11 generated)
rwlbuis
PTAL. I checked in CSSPropertyParser.cpp that no other "double" uses of validUnit are done.
5 years, 10 months ago (2015-02-11 21:17:15 UTC) #2
rwlbuis
5 years, 10 months ago (2015-02-18 16:54:00 UTC) #4
rune
https://codereview.chromium.org/912383003/diff/1/LayoutTests/fast/css/calc-in-text-shadow-property.html File LayoutTests/fast/css/calc-in-text-shadow-property.html (right): https://codereview.chromium.org/912383003/diff/1/LayoutTests/fast/css/calc-in-text-shadow-property.html#newcode1 LayoutTests/fast/css/calc-in-text-shadow-property.html:1: <style> Use <!DOCTYPE html> and use some text saying ...
5 years, 10 months ago (2015-02-18 19:09:54 UTC) #5
rwlbuis
On 2015/02/18 19:09:54, rune wrote: > https://codereview.chromium.org/912383003/diff/1/LayoutTests/fast/css/calc-in-text-shadow-property.html > File LayoutTests/fast/css/calc-in-text-shadow-property.html (right): > > https://codereview.chromium.org/912383003/diff/1/LayoutTests/fast/css/calc-in-text-shadow-property.html#newcode1 > ...
5 years, 10 months ago (2015-02-18 19:44:15 UTC) #6
rune
On 2015/02/18 at 19:44:15, rob.buis wrote: > On 2015/02/18 19:09:54, rune wrote: > > https://codereview.chromium.org/912383003/diff/1/LayoutTests/fast/css/calc-in-text-shadow-property.html ...
5 years, 10 months ago (2015-02-18 21:35:54 UTC) #7
rwlbuis
On 2015/02/18 21:35:54, rune wrote: > > Good point! Indeed it crashed, I changed the ...
5 years, 10 months ago (2015-02-18 21:41:59 UTC) #8
rune
lgtm
5 years, 10 months ago (2015-02-18 21:58:57 UTC) #9
rwlbuis
On 2015/02/18 21:58:57, rune wrote: > lgtm Thanks!
5 years, 10 months ago (2015-02-18 22:08:39 UTC) #10
rwlbuis
@timloh seems like rune's okay is not enough...
5 years, 10 months ago (2015-02-18 23:13:19 UTC) #15
Timothy Loh
+alancutter for calc changes
5 years, 10 months ago (2015-02-18 23:36:32 UTC) #17
alancutter1
lgtm with nit. https://codereview.chromium.org/912383003/diff/40001/Source/core/css/parser/CSSPropertyParser.cpp File Source/core/css/parser/CSSPropertyParser.cpp (right): https://codereview.chromium.org/912383003/diff/40001/Source/core/css/parser/CSSPropertyParser.cpp#newcode5555 Source/core/css/parser/CSSPropertyParser.cpp:5555: || (!m_parsedCalculation && !validUnit(val, FLength | ...
5 years, 10 months ago (2015-02-19 00:36:37 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912383003/40001
5 years, 10 months ago (2015-02-19 03:25:56 UTC) #20
alancutter (OOO until 2018)
On 2015/02/19 03:30:49, alancutter1 wrote: > The CQ bit was unchecked by mailto:alancutter@google.com CQ started ...
5 years, 10 months ago (2015-02-19 03:46:38 UTC) #22
rwlbuis
On 2015/02/19 03:46:38, alancutter wrote: > On 2015/02/19 03:30:49, alancutter1 wrote: > > The CQ ...
5 years, 10 months ago (2015-02-19 15:32:09 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/912383003/60001
5 years, 10 months ago (2015-02-19 16:21:25 UTC) #26
commit-bot: I haz the power
5 years, 10 months ago (2015-02-19 21:01:27 UTC) #27
Message was sent while issue was closed. 
Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190493

Powered by Google App Engine
This is Rietveld 408576698