Revert of small tidy of benchmarkstream (patchset #2 id:20001 of https://codereview.chromium.org/1395703002/ )

Reason for revert:
Breaks visualbench

Original issue's description:
> small tidy of benchmarkstream
>
> BUG=skia:
>
> Committed: https://skia.googlesource.com/skia/+/691b6af907e55250a29a7a2a346b63c2026011c3

TBR=robertphillips@google.com,joshualitt@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review URL: https://codereview.chromium.org/1392833006

6 files changed, 49 insertions, 40 deletions

diff --git a/tools/VisualBench/TimingStateMachine.cpp b/tools/VisualBench/TimingStateMachine.cpp
index 85aab42ba9..ae03f5d188 100644
--- a/tools/VisualBench/TimingStateMachine.cpp
+++ b/tools/VisualBench/TimingStateMachine.cpp
@@ -91,7 +91,9 @@ void TimingStateMachine::recordMeasurement() {
     fLastMeasurement = this->elapsed() / (FLAGS_frames * fLoops);
 }
 
-void TimingStateMachine::nextBenchmark() {
+void TimingStateMachine::nextBenchmark(SkCanvas* canvas, Benchmark* benchmark) {
+    benchmark->postDraw(canvas);
+    benchmark->perCanvasPostDraw(canvas);
     fLoops = 1;
     fInnerState = kTuning_InnerState;
     fState = kPreWarm_State;
diff --git a/tools/VisualBench/TimingStateMachine.h b/tools/VisualBench/TimingStateMachine.h
index 3f40b6d31f..22150599ab 100644
--- a/tools/VisualBench/TimingStateMachine.h
+++ b/tools/VisualBench/TimingStateMachine.h
@@ -38,9 +38,10 @@ public:
     ParentEvents nextFrame(bool preWarmBetweenSamples);
 
     /*
-     * The caller should call this when they are ready to move to the next benchmark.
+     * The caller should call this when they are ready to move to the next benchmark.  The caller
+     * must call this with the *last* benchmark so post draw hooks can be invoked
      */
-    void nextBenchmark();
+    void nextBenchmark(SkCanvas*, Benchmark*);
 
     /*
      * When TimingStateMachine returns kTimingFinished_ParentEvents, then the owner can call
diff --git a/tools/VisualBench/VisualBenchmarkStream.cpp b/tools/VisualBench/VisualBenchmarkStream.cpp
index 49bafbfcee..c520eeed05 100644
--- a/tools/VisualBench/VisualBenchmarkStream.cpp
+++ b/tools/VisualBench/VisualBenchmarkStream.cpp
@@ -74,9 +74,6 @@ VisualBenchmarkStream::VisualBenchmarkStream()
             }
         }
     }
-
-    // seed with an initial benchmark
-    this->next();
 }
 
 bool VisualBenchmarkStream::ReadPicture(const char* path, SkAutoTUnref<SkPicture>* pic) {
@@ -101,24 +98,23 @@ bool VisualBenchmarkStream::ReadPicture(const char* path, SkAutoTUnref<SkPicture
 }
 
 Benchmark* VisualBenchmarkStream::next() {
-    Benchmark* bench;
     if (!fIsWarmedUp) {
         fIsWarmedUp = true;
-        bench = new WarmupBench;
-    } else {
-        // skips non matching benches
-        while ((bench = this->innerNext()) &&
-               (SkCommandLineFlags::ShouldSkip(FLAGS_match, bench->getUniqueName()) ||
-                !bench->isSuitableFor(Benchmark::kGPU_Backend))) {
-            bench->unref();
-        }
-    }
-    if (bench && FLAGS_cpu) {
-        bench = new CpuWrappedBenchmark(bench);
+        return new WarmupBench;
     }
 
-    fBenchmark.reset(bench);
-    return fBenchmark;
+    Benchmark* bench;
+
+    // skips non matching benches
+    while ((bench = this->innerNext()) &&
+           (SkCommandLineFlags::ShouldSkip(FLAGS_match, bench->getUniqueName()) ||
+            !bench->isSuitableFor(Benchmark::kGPU_Backend))) {
+        bench->unref();
+    }
+    if (FLAGS_cpu) {
+        return new CpuWrappedBenchmark(bench);
+    }
+    return bench;
 }
 
 Benchmark* VisualBenchmarkStream::innerNext() {
diff --git a/tools/VisualBench/VisualBenchmarkStream.h b/tools/VisualBench/VisualBenchmarkStream.h
index 89ca632821..9e5d4590cb 100644
--- a/tools/VisualBench/VisualBenchmarkStream.h
+++ b/tools/VisualBench/VisualBenchmarkStream.h
@@ -23,7 +23,6 @@ public:
     static bool ReadPicture(const char* path, SkAutoTUnref<SkPicture>* pic);
 
     Benchmark* next();
-    Benchmark* current() { return fBenchmark.get(); }
 
 private:
     Benchmark* innerNext();
@@ -31,7 +30,6 @@ private:
     const BenchRegistry* fBenches;
     const skiagm::GMRegistry* fGMs;
     SkTArray<SkString> fSKPs;
-    SkAutoTUnref<Benchmark> fBenchmark;
 
     const char* fSourceType;  // What we're benching: bench, GM, SKP, ...
     const char* fBenchType;   // How we bench it: micro, playback, ...
diff --git a/tools/VisualBench/VisualStreamTimingModule.cpp b/tools/VisualBench/VisualStreamTimingModule.cpp
index 4148282ce6..db75890d11 100644
--- a/tools/VisualBench/VisualStreamTimingModule.cpp
+++ b/tools/VisualBench/VisualStreamTimingModule.cpp
@@ -16,43 +16,52 @@ VisualStreamTimingModule::VisualStreamTimingModule(VisualBench* owner, bool preW
     fBenchmarkStream.reset(new VisualBenchmarkStream);
 }
 
+bool VisualStreamTimingModule::nextBenchmarkIfNecessary(SkCanvas* canvas) {
+    if (fBenchmark) {
+        return true;
+    }
+
+    fBenchmark.reset(fBenchmarkStream->next());
+    if (!fBenchmark) {
+        return false;
+    }
+
+    fOwner->clear(canvas, SK_ColorWHITE, 2);
+
+    fBenchmark->delayedSetup();
+    fBenchmark->preTimingHooks(canvas);
+    return true;
+}
+
 void VisualStreamTimingModule::draw(SkCanvas* canvas) {
-    if (!fBenchmarkStream->current()) {
-        // this should never happen but just to be safe
+    if (!this->nextBenchmarkIfNecessary(canvas)) {
+        SkDebugf("Exiting VisualBench successfully\n");
+        fOwner->closeWindow();
         return;
     }
 
     if (fReinitializeBenchmark) {
         fReinitializeBenchmark = false;
-        fBenchmarkStream->current()->preTimingHooks(canvas);
+        fBenchmark->preTimingHooks(canvas);
     }
 
-    this->renderFrame(canvas, fBenchmarkStream->current(), fTSM.loops());
+    this->renderFrame(canvas, fBenchmark, fTSM.loops());
     fOwner->present();
     TimingStateMachine::ParentEvents event = fTSM.nextFrame(fPreWarmBeforeSample);
     switch (event) {
         case TimingStateMachine::kReset_ParentEvents:
-            fBenchmarkStream->current()->postTimingHooks(canvas);
+            fBenchmark->postTimingHooks(canvas);
             fOwner->reset();
             fReinitializeBenchmark = true;
             break;
         case TimingStateMachine::kTiming_ParentEvents:
             break;
         case TimingStateMachine::kTimingFinished_ParentEvents:
-            fBenchmarkStream->current()->postTimingHooks(canvas);
+            fBenchmark->postTimingHooks(canvas);
             fOwner->reset();
-            if (this->timingFinished(fBenchmarkStream->current(), fTSM.loops(),
-                                     fTSM.lastMeasurement())) {
-                fTSM.nextBenchmark();
-                if (!fBenchmarkStream->next()) {
-                    SkDebugf("Exiting VisualBench successfully\n");
-                    fOwner->closeWindow();
-                } else {
-                    fOwner->clear(canvas, SK_ColorWHITE, 2);
-
-                    fBenchmarkStream->current()->delayedSetup();
-                    fBenchmarkStream->current()->preTimingHooks(canvas);
-                }
+            if (this->timingFinished(fBenchmark, fTSM.loops(), fTSM.lastMeasurement())) {
+                fTSM.nextBenchmark(canvas, fBenchmark);
+                fBenchmark.reset(nullptr);
             } else {
                 fReinitializeBenchmark = true;
             }
diff --git a/tools/VisualBench/VisualStreamTimingModule.h b/tools/VisualBench/VisualStreamTimingModule.h
index b84f04f4bb..ac06ed4307 100644
--- a/tools/VisualBench/VisualStreamTimingModule.h
+++ b/tools/VisualBench/VisualStreamTimingModule.h
@@ -31,8 +31,11 @@ private:
     // subclasses should return true to advance the stream
     virtual bool timingFinished(Benchmark*, int loops, double measurement)=0;
 
+    bool nextBenchmarkIfNecessary(SkCanvas*);
+
     TimingStateMachine fTSM;
     SkAutoTDelete<VisualBenchmarkStream> fBenchmarkStream;
+    SkAutoTUnref<Benchmark> fBenchmark;
     bool fReinitializeBenchmark;
     bool fPreWarmBeforeSample;