Regression:NTP crashes on opening Devtools
Reported by
shruti.j...@etouch.net,
Oct 24 2017
|
|||||||||||||
Issue descriptionChrome Version:63.0.3239.16 (Official Build) (64-bit) (cohort: 62_62_win)429b133a5af5a24d0d81d4a8b6a8f8d7d3385f13-refs/branch-heads/3239@{#157} OS: Win(7,8,10), Mac(10.12.6) and Linux(14.04 LTS). Steps to reproduce: 1.Launch chrome, go to NTP and open devtools. 2.Observe. Actual Result: Tab crash is seen on NTP Expected Result: Tab should not crash . Uploaded Crash Report ID eea2ac4d6995df0b (Local Crash ID: 7fef8b8d-883d-4418-95d6-946569e12aab)
,
Oct 24 2017
,
Oct 24 2017
This is regression issue broken in ‘M-63’ and below is the bisect info. Manual Bisect Good Build: 63.0.3239.15 Bad Build: 63.0.3239.16 CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/63.0.3239.15..63.0.3239.16?pretty=fuller&n=10000 Suspect:https://chromium.googlesource.com/chromium/src/+/9fd2dccce2d81546f8abf8a518b4036850cc21c5 @Naina Raisinghani: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Note:Cannot Narrow down using bisect script as the issue is broken in Branch and Bisect script is generating error when executed. Thank You!
,
Oct 24 2017
Tagging this issue with Blocker label, This is blocking todays dev candidate. Can this be addressed. Thanks.!
,
Oct 24 2017
This is happening on any page, not just NTP..Just launching Dev tools crashes the page
,
Oct 24 2017
,
Oct 24 2017
+ dgozman@, could you ptal as suspected CL :https://chromium.googlesource.com/chromium/src/+/9fd2dccce2d81546f8abf8a518b4036850cc21c5 (63.0.3239.15) author is in Australia?
,
Oct 24 2017
I am reverting locally [1] and [2] to see whether it fixes the crash. [1] https://chromium.googlesource.com/chromium/src/+/9fd2dccce2d81546f8abf8a518b4036850cc21c5 [2] https://chromium.googlesource.com/chromium/src/+/243e0ebdc75cb7c74afe6c70c501aaedd3f949e8
,
Oct 24 2017
Thank you dgozman@. Please merge both reverts to M63 branch 3239 if it fixes the crash and safe to merge. We're blocking today's dev release and plan for dev release tomorrow (if this bug gets fixed today)
,
Oct 24 2017
Confirmed locally that reverting both fixes the crash. Requesting approval to revert in 63 branch.
,
Oct 24 2017
Approving merge to M63 branch 3239 based on comment #11 and per offline chat with dgozman@ and abdulsyed@. Please merge ASAP. Thank you.
,
Oct 24 2017
dgozman@ and nainar@ curious to know what are the implications of reverting the CL's on M63, Since the CL's are related to "Stability-Memory-AddressSanitizer". Please let us know if we plan to address this during M63 Beta phase.
,
Oct 24 2017
re #c13: that's a good question. I guess the correct fix would include the original revert and something else? Note that ToT has both patches but does not exhibit the crash. Perhaps, there is another change which mitigates the crash and we could merge it?
,
Oct 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6142075c8c7d2100666593f8a2abd94ed9710e6 commit d6142075c8c7d2100666593f8a2abd94ed9710e6 Author: Dmitry Gozman <dgozman@chromium.org> Date: Tue Oct 24 18:28:10 2017 Revert "Fix compilation error on Beta" This reverts commit 9fd2dccce2d81546f8abf8a518b4036850cc21c5. Reason for revert: crashes DevTools on any site. See crbug.com/777720 . Bug: 777720 Original change's description: > Fix compilation error on Beta > > Breaks beta: https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/linux64%20beta/builds/2020 > > Caused by change to CSSParserImpl.cpp (https://chromium.googlesource.com/chromium/src/+/243e0ebdc75cb7c74afe6c70c501aaedd3f949e8) > > TBR=shend@chromium.org > > Bug: > Change-Id: I79dc9dcdd199d7ad1e43077c689b9d4f6400ea3a > Reviewed-on: https://chromium-review.googlesource.com/732332 > Reviewed-by: nainar <nainar@chromium.org> > Cr-Commit-Position: refs/branch-heads/3239@{#147} > Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} TBR=nainar@chromium.org,shend@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: I701b89a3229252205318ca059c7501d74f97065a Reviewed-on: https://chromium-review.googlesource.com/735217 Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#181} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [modify] https://crrev.com/d6142075c8c7d2100666593f8a2abd94ed9710e6/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp
,
Oct 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c298e7c357bcbc2554fe5d132da15566c028b8ff commit c298e7c357bcbc2554fe5d132da15566c028b8ff Author: Dmitry Gozman <dgozman@chromium.org> Date: Tue Oct 24 18:29:08 2017 Revert "Revert implementing Lazy Parsing for Pseudo attributes (before/after)" This reverts commit 243e0ebdc75cb7c74afe6c70c501aaedd3f949e8. Reason for revert: crashes DevTools on any site. See crbug.com/777720 . Bug: 777720 Original change's description: > Revert implementing Lazy Parsing for Pseudo attributes (before/after) > > This is due to the change being crashy > > Revert CSS Parser: Lazy Parsing for Pseudo attributes (before/after) > > Revert Move has_before_or_after_ from CSSLazyParsingState to CSSLazyPropertyParserImpl > > TBR=nainar@google.com, shend@chromium.org > > (cherry picked from commit 7aa7dab403ad24848afa2dfa57cb344427b31a8d) > > Bug: 774061 > Change-Id: I529f56c8751b11305cc23d506b8d4f4df476ae2a > Reviewed-on: https://chromium-review.googlesource.com/729600 > Commit-Queue: nainar <nainar@chromium.org> > Reviewed-by: nainar <nainar@chromium.org> > Cr-Original-Commit-Position: refs/heads/master@{#510696} > Reviewed-on: https://chromium-review.googlesource.com/732732 > Cr-Commit-Position: refs/branch-heads/3239@{#143} > Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} TBR=nainar@chromium.org,shend@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 774061 Change-Id: I9383956cec272b8fea8dc694902e1581514f1ab9 Reviewed-on: https://chromium-review.googlesource.com/734665 Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Cr-Commit-Position: refs/branch-heads/3239@{#182} Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578} [add] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/LayoutTests/fast/css/lazy-parsing-pseudo-attribute.html [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/RuleFeature.cpp [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/RuleFeature.h [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/RuleSet.h [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/StyleEngine.h [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/StylePropertySet.h [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/StyleRule.h [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.h [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/parser/CSSLazyParsingTest.cpp [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.cpp [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/parser/CSSLazyPropertyParserImpl.h [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp [modify] https://crrev.com/c298e7c357bcbc2554fe5d132da15566c028b8ff/third_party/WebKit/Source/core/dom/Document.cpp
,
Oct 24 2017
Hi, I am currently in the US timezone but I just saw this as I am AFK due to CDS. I will let dgozman@ proceed with the revert since they have more context. Curious so as to why this revert is causing crashes in 63 but not 64 since https://chromium.googlesource.com/chromium/src/+/243e0ebdc75cb7c74afe6c70c501aaedd3f949e8 landed in Dev and hasn't caused issues there and https://chromium.googlesource.com/chromium/src/+/9fd2dccce2d81546f8abf8a518b4036850cc21c5 was a fix necessary due to a merge conflict? This revert will cause crbug.com/774061 to rise again. But since that is only when people have experimental web platform features enabled, this is more severe. Re #13. The CLs are only to pull out code for --enable-experimental-web-platform-features so it shouldn't be a problem.
,
Oct 24 2017
Thank you for landing the reverts in dgozman@ I will try to see what went wrong with https://chromium.googlesource.com/chromium/src/+/9fd2dccce2d81546f8abf8a518b4036850cc21c5 since it is the only difference between 64 and 63 (pre your reverts). I think given the volatility this change has caused not touching 63 is the best approach. Terribly sorry for the inconvenience!
,
Oct 24 2017
Note that ToT also has https://chromium.googlesource.com/chromium/src/+/ab1291fb2b682683463ca51ed3e616af39c0926f which might be mitigating the crash?
,
Oct 24 2017
Yup I am looking into Lazy + streaming parser interaction to see what's going on. Just for my own curiosity. Marking this as fixed in the meanwhile. And marking you as Owner so that you get appropriate credit.
,
Oct 24 2017
Able to reproduce the issue on Chrome 63.0.3239.17/CrOS 10032.14.0 - Daisy Crash ID : 235c13aff0711d49
,
Oct 24 2017
gkihumba@ Is this post the revert in comments 15 and 16?
,
Oct 25 2017
Note: Retested the above issue on Dev(PGO) 63.0.3239.18 on Win(7,8,10), Mac(10.12.6,10.13.1) and Linux(14.04 LTS) and fix is working as intended.Hence adding TE-Verified label. Attached the screenshot for reference. Thank you! |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by ranjitkan@chromium.org
, Oct 24 2017