[heap-profiler] Report finished progress only once.

This fixes HeapSnapshotGenerator::SetProgressTotal so that
ProgressReport is called with finished flag only once.

The DevTools front-end assumes that progress with finished flag is
reported only once.

Change-Id: Iad958478aa8ad27a520cb491419e521027967754
Reviewed-on: https://chromium-review.googlesource.com/949224
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51767}

src/profiler/heap-snapshot-generator.cc

@@ -2485,9 +2485,15 @@ bool HeapSnapshotGenerator::ProgressReport(bool force) {
 void HeapSnapshotGenerator::SetProgressTotal(int iterations_count) {
   if (control_ == nullptr) return;
   HeapIterator iterator(heap_, HeapIterator::kFilterUnreachable);
-  progress_total_ = iterations_count * (
-      v8_heap_explorer_.EstimateObjectsCount(&iterator) +
-      dom_explorer_.EstimateObjectsCount());
+  // The +1 ensures that intermediate ProgressReport calls will never signal
+  // that the work is finished (i.e. progress_counter_ == progress_total_).
+  // Only the forced ProgressReport() at the end of GenerateSnapshot()
+  // should signal that the work is finished because signalling finished twice
+  // breaks the DevTools frontend.
+  progress_total_ =
+      iterations_count * (v8_heap_explorer_.EstimateObjectsCount(&iterator) +
+                          dom_explorer_.EstimateObjectsCount()) +
+      1;
   progress_counter_ = 0;
 }
 

test/cctest/test-heap-profiler.cc

@@ -1430,10 +1430,18 @@ namespace {
 class TestActivityControl : public v8::ActivityControl {
  public:
   explicit TestActivityControl(int abort_count)
-      : done_(0), total_(0), abort_count_(abort_count) {}
+      : done_(0),
+        total_(0),
+        abort_count_(abort_count),
+        reported_finish_(false) {}
   ControlOption ReportProgressValue(int done, int total) {
     done_ = done;
     total_ = total;
+    CHECK_LE(done_, total_);
+    if (done_ == total_) {
+      CHECK(!reported_finish_);
+      reported_finish_ = true;
+    }
     return --abort_count_ != 0 ? kContinue : kAbort;
   }
   int done() { return done_; }
@@ -1443,6 +1451,7 @@ class TestActivityControl : public v8::ActivityControl {
   int done_;
   int total_;
   int abort_count_;
+  bool reported_finish_;
 };
 
 }  // namespace
@@ -1471,6 +1480,16 @@ TEST(TakeHeapSnapshotAborting) {
   CHECK_GT(control.total(), 0);
 }
 
+TEST(TakeHeapSnapshotReportFinishOnce) {
+  LocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+  v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
+  TestActivityControl control(-1);
+  const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot(&control);
+  CHECK(ValidateSnapshot(snapshot));
+  CHECK_EQ(control.total(), control.done());
+  CHECK_GT(control.total(), 0);
+}
 
 namespace {