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

Issue 919273002: Revert of Revert of Add test_runner hook to dump drag image. (Closed)

Created:
5 years, 10 months ago by jackhou1
Modified:
5 years, 10 months ago
Reviewers:
tkent, Justin Novosad
CC:
chromium-reviews, darin-cc_chromium.org, mkwst+moarreviews-shell_chromium.org, jam, jochen+watch_chromium.org, mlamouri+watch-content_chromium.org, chrome-apps-syd-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Revert of Add test_runner hook to dump drag image. (patchset #1 id:1 of https://codereview.chromium.org/925743002/) Reason for revert: Reverting this breaks the blink side CL that depended on the new test_runner hook: https://codereview.chromium.org/886323005/ http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux/builds/44149 Fix is here: https://codereview.chromium.org/918313003/ I think it was failing CQ for unrelated reasons, so reverting this. Original issue's description: > Revert of Add test_runner hook to dump drag image. (patchset #3 id:40001 of https://codereview.chromium.org/904833004/) > > Reason for revert: > Caused pixel test flakiness. > > Original issue's description: > > Add test_runner hook to dump drag image. > > > > This is used by: > > https://codereview.chromium.org/886323005/ > > > > BUG=451759 > > > > Committed: https://crrev.com/3726d659908604350bc03ef02d1ccc5cdc097dd5 > > Cr-Commit-Position: refs/heads/master@{#315915} > > TBR=junov@chromium.org,jackhou@chromium.org > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=451759 > > Committed: https://crrev.com/29881876aff6c4639ac9378a2911be87b73c8a0a > Cr-Commit-Position: refs/heads/master@{#316136} BUG=451759, 458077

Patch Set 1 #

Patch Set 2 : Patch in fix from https://codereview.chromium.org/918313003/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -1 line) Patch
M content/shell/renderer/test_runner/test_runner.h View 1 3 chunks +10 lines, -0 lines 0 comments Download
M content/shell/renderer/test_runner/test_runner.cc View 1 6 chunks +17 lines, -0 lines 0 comments Download
M content/shell/renderer/test_runner/web_test_proxy.h View 1 3 chunks +3 lines, -1 line 0 comments Download
M content/shell/renderer/test_runner/web_test_proxy.cc View 1 3 chunks +10 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
jackhou1
Created Revert of Revert of Add test_runner hook to dump drag image.
5 years, 10 months ago (2015-02-13 03:03:48 UTC) #1
tkent
Not lgtm. We already reverted, and I'll add fast/images/drag-image.html to TestExpectations. It's important to make ...
5 years, 10 months ago (2015-02-13 03:07:03 UTC) #2
jackhou1
On 2015/02/13 03:07:03, tkent wrote: > Not lgtm. > > We already reverted, and I'll ...
5 years, 10 months ago (2015-02-13 03:23:14 UTC) #3
tkent
On 2015/02/13 03:23:14, jackhou1 wrote: > I've removed the TBR, NOTRY etc from this CL. ...
5 years, 10 months ago (2015-02-13 03:39:18 UTC) #4
jackhou1
5 years, 10 months ago (2015-02-13 04:53:32 UTC) #5
On 2015/02/13 03:39:18, tkent wrote:
> On 2015/02/13 03:23:14, jackhou1 wrote:
> > I've removed the TBR, NOTRY etc from this CL. Along with the Patch Set 2, it
> > should be everything that's needed to fix the test. Could you review it
again?
> 
> The CL looks a revert, and it's very confusing.
> 
> > If you prefer, I can instead put together a new CL, or reopen the previous
one
> > (http://crrev.com/886323005).
> 
> I prefer this way.  Either of new CL or uploading new Patch Set to the
original
> CL is ok.

New CL is here: https://codereview.chromium.org/916893003/

Powered by Google App Engine
This is Rietveld 408576698