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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2179563002: Revert of Remove platform/text/ParserUtilities.h (Closed)

Created:
4 years, 5 months ago by Mark P
Modified:
4 years, 5 months ago
Reviewers:
Stephen Chennney, pdr., fs
CC:
blink-reviews, chromium-reviews, krit, f(malita), gyuyoung2, kouhei+svg_chromium.org, pdr+svgwatchlist_chromium.org, rwlbuis, Stephen Chennney
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Remove platform/text/ParserUtilities.h (patchset #2 id:20001 of https://codereview.chromium.org/2176623003/ ) Reason for revert: Likely cause of failures: --- https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/67858 unexpected_failures: fast/forms/color/color-suggestion-picker-appearance-zoom125.html fast/forms/color/color-suggestion-picker-with-scrollbar-appearance.html fast/forms/color/color-suggestion-picker-appearance-zoom200.html all with the message image diff --- --- also bot: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.11%20%28dbg%29/builds/3384 http/tests/images/restyle-decode-error.html in which case there is a renderer crash in this test --- also some other bots for tests fast/forms/color/color-suggestion-picker-appearance.html fast/forms/color/color-suggestion-picker-one-row-appearance.html fast/forms/color/color-suggestion-picker-two-row-appearance.html Original issue's description: > Remove platform/text/ParserUtilities.h > > platform/ParsingUtilities.h caters to the same needs, so transition > users of skipString(...) to skipToken(...) and remove > platform/text/ParserUtilities.h. > > Committed: https://crrev.com/39cc523b5dbf9d2ddad9483cb27fab8b97e53ec1 > Cr-Commit-Position: refs/heads/master@{#407187} TBR=schenney@chromium.org,pdr@chromium.org,fs@opera.com # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Committed: https://crrev.com/c23c19ccc51c344a43857ddb667db4104b1c50c2 Cr-Commit-Position: refs/heads/master@{#407204}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+107 lines, -21 lines) Patch
M third_party/WebKit/Source/core/svg/SVGPreserveAspectRatio.cpp View 3 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGTransformList.cpp View 3 chunks +14 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGViewSpec.cpp View 7 chunks +14 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/svg/SVGZoomAndPan.cpp View 2 chunks +6 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/platform/blink_platform.gypi View 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/Source/platform/text/ParserUtilities.h View 1 chunk +68 lines, -0 lines 0 comments Download

Messages

Total messages: 15 (3 generated)
Mark P
Created Revert of Remove platform/text/ParserUtilities.h
4 years, 5 months ago (2016-07-22 18:10:38 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2179563002/1
4 years, 5 months ago (2016-07-22 18:11:05 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 5 months ago (2016-07-22 18:11:53 UTC) #5
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/c23c19ccc51c344a43857ddb667db4104b1c50c2 Cr-Commit-Position: refs/heads/master@{#407204}
4 years, 5 months ago (2016-07-22 18:13:42 UTC) #7
fs
On 2016/07/22 at 18:10:38, mpearson wrote: > Created Revert of Remove platform/text/ParserUtilities.h I would be ...
4 years, 5 months ago (2016-07-22 18:18:15 UTC) #8
Mark P
On 2016/07/22 18:18:15, fs wrote: > On 2016/07/22 at 18:10:38, mpearson wrote: > > Created ...
4 years, 5 months ago (2016-07-22 18:26:06 UTC) #9
fs
On 2016/07/22 at 18:26:06, mpearson wrote: > On 2016/07/22 18:18:15, fs wrote: > > On ...
4 years, 5 months ago (2016-07-22 18:35:38 UTC) #10
Mark P
On 2016/07/22 18:35:38, fs wrote: > On 2016/07/22 at 18:26:06, mpearson wrote: > > On ...
4 years, 5 months ago (2016-07-22 18:46:34 UTC) #11
fs
On 2016/07/22 at 18:46:34, mpearson wrote: > On 2016/07/22 18:35:38, fs wrote: > > On ...
4 years, 5 months ago (2016-07-22 18:49:55 UTC) #12
Mark P
On 2016/07/22 18:49:55, fs wrote: > On 2016/07/22 at 18:46:34, mpearson wrote: > > On ...
4 years, 5 months ago (2016-07-22 19:19:45 UTC) #13
pdr.
On 2016/07/22 at 19:19:45, mpearson wrote: > On 2016/07/22 18:49:55, fs wrote: > > On ...
4 years, 5 months ago (2016-07-22 19:24:52 UTC) #14
Mark P
4 years, 5 months ago (2016-07-22 20:38:41 UTC) #15
Message was sent while issue was closed. 
On 2016/07/22 19:24:52, pdr. wrote:
> On 2016/07/22 at 19:19:45, mpearson wrote:
> > On 2016/07/22 18:49:55, fs wrote:
> > > On 2016/07/22 at 18:46:34, mpearson wrote:
> > > > On 2016/07/22 18:35:38, fs wrote:
> > > > > On 2016/07/22 at 18:26:06, mpearson wrote:
> > > > > > On 2016/07/22 18:18:15, fs wrote:
> > > > > > > On 2016/07/22 at 18:10:38, mpearson wrote:
> > > > > > > > Created Revert of Remove platform/text/ParserUtilities.h
> > > > > > > 
> > > > > > > I would be more suspicious of:
> > > > > > > 
> > > > > > >
> > > > >
> > >
>
https://chromium.googlesource.com/chromium/src/+/e82c4e1b9de3b189c85bccff63d4...
> > > > > > > 
> > > > > > > since it appears to replace "..." with a proper ellipsis, which
all
> the
> > > > > three
> > > > > > > tests show differences from:
> > > > > > > 
> > > > > > >
> > > > >
> > >
>
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux/678...
> > > > > > 
> > > > > > Good point.  I didn't actually see where the images diff.
> > > > > > 
> > > > > > I'll check out some of the webkit failures to see if that changelist
> is
> > > > > consistent with the other failures.  If so, I'll revert it.  Apologies
> if
> > > this
> > > > > revert turned out to be incorrect.  (I was looking for webkit changes
> for
> > > webkit
> > > > > failures...)
> > > > > 
> > > > >
> > >
>
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.10/build...
> > > > > 
> > > > > Appears to have more failures of a similar kind (and the reverted CL
> isn't
> > > in
> > > > > the range built.)
> > > > 
> > > > You are correct.  One tester has made it to a round with my revert CL
in,
> and
> > > the tests are still failing.  Apologies.
> > > > 
> > > > --mark
> > > 
> > >
>
https://chromium.googlesource.com/chromium/src/+/520519605b961a7d979bc0eaa3c0...
> > > might be the culprit for http/tests/images/restyle-decode-error.html
(based
> on
> > > stack in
> > >
>
https://storage.googleapis.com/chromium-layout-test-archives/WebKit_Linux_ASA...)
> > 
> > I believe it.  I found some bots where this was the only new failure
compared
> to previous runs, and the patch you point out is in the blamelist.  I'll
revert
> this one too.  Oy.
> > 
> > By the way, for my edification, how do you get to those storage.googleapis
> URLs?  Being able to get there easily would really help me being able to track
> these things down myself.
> > 
> > --mark
> 
> To get the layout test results, just click the "layout_test_results" link
under
> the "archive_webkit_tests_results" header (which immediately follows the
> "webkit_tests (with patch)" step).

Oh, so easy.  I never thought to look at the other (non-broken) steps.  (I
assumed they were going to be irrelevant.)

--mark

Powered by Google App Engine
This is Rietveld 408576698