Add restart_id to BeginFrameSource::source_id_.

When running with --enable-viz the display compositor is relocated to
the GPU process. When the GPU process crashes the display compositor is
restarted and the BeginFrameSource for each viz::Display is recreated.
This leads to a problem where |source_id_| gets reused and
BeginFrameObservers DCHECK on a BeginFrameArgs with the same source_id
but reset sequence_number.

Combine a 32 bit atomic sequence and 32 bit |restart_id| to get the
BeginFrameSource::source_id_. Only BeginFrameSources used in the GPU
need this functionality, so |restart_id| is only plumbed through for
DelayBasedBeginFrameSource. Other BeginFrameSources use
kNotRestartableId. It will be used in a followup CL from
GpuDisplayProvider.

Bug: 782268
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I1ef924f43e5fc21a70a0dc96a756929db60626af
Reviewed-on: https://chromium-review.googlesource.com/755893
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Brian Anderson <brianderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517097}

cc/scheduler/scheduler_unittest.cc

@@ -272,9 +272,10 @@ class SchedulerTest : public testing::Test {
     fake_external_begin_frame_source_.reset(
         new viz::FakeExternalBeginFrameSource(1.0, false));
     fake_external_begin_frame_source_->SetClient(client_.get());
-    synthetic_frame_source_.reset(new viz::DelayBasedBeginFrameSource(
+    synthetic_frame_source_ = std::make_unique<viz::DelayBasedBeginFrameSource>(
         std::make_unique<viz::FakeDelayBasedTimeSource>(now_src_.get(),
-                                                        task_runner_.get())));
+                                                        task_runner_.get()),
+        viz::BeginFrameSource::kNotRestartableId);
     switch (bfs_type) {
       case EXTERNAL_BFS:
         frame_source = fake_external_begin_frame_source_.get();

components/viz/common/frame_sinks/begin_frame_source.cc

@@ -22,7 +22,16 @@ namespace viz {
 namespace {
 // kDoubleTickDivisor prevents the SyntheticBFS from sending BeginFrames too
 // often to an observer.
-static const double kDoubleTickDivisor = 2.0;
+constexpr double kDoubleTickDivisor = 2.0;
+
+base::AtomicSequenceNumber g_next_source_id;
+
+// Generates a source_id with upper 32 bits from |restart_id| and lower 32 bits
+// from an atomic sequence.
+uint64_t GenerateSourceId(uint32_t restart_id) {
+  return static_cast<uint64_t>(restart_id) << 32 | g_next_source_id.GetNext();
+}
+
 }  // namespace
 
 // BeginFrameObserverBase -------------------------------------------------
@@ -59,13 +68,12 @@ void BeginFrameObserverBase::AsValueInto(
 }
 
 // BeginFrameSource -------------------------------------------------------
-namespace {
-static base::AtomicSequenceNumber g_next_source_id;
-}  // namespace
 
-// TODO(kylechar): Use the higher 32 bits of |source_id_| for process restart
-// id.
-BeginFrameSource::BeginFrameSource() : source_id_(g_next_source_id.GetNext()) {}
+// static
+constexpr uint32_t BeginFrameSource::kNotRestartableId;
+
+BeginFrameSource::BeginFrameSource(uint32_t restart_id)
+    : source_id_(GenerateSourceId(restart_id)) {}
 
 BeginFrameSource::~BeginFrameSource() = default;
 
@@ -76,17 +84,24 @@ void BeginFrameSource::AsValueInto(
 }
 
 // StubBeginFrameSource ---------------------------------------------------
+StubBeginFrameSource::StubBeginFrameSource()
+    : BeginFrameSource(kNotRestartableId) {}
+
 bool StubBeginFrameSource::IsThrottled() const {
   return true;
 }
 
 // SyntheticBeginFrameSource ----------------------------------------------
+SyntheticBeginFrameSource::SyntheticBeginFrameSource(uint32_t restart_id)
+    : BeginFrameSource(restart_id) {}
+
 SyntheticBeginFrameSource::~SyntheticBeginFrameSource() = default;
 
 // BackToBackBeginFrameSource ---------------------------------------------
 BackToBackBeginFrameSource::BackToBackBeginFrameSource(
     std::unique_ptr<DelayBasedTimeSource> time_source)
-    : time_source_(std::move(time_source)),
+    : SyntheticBeginFrameSource(kNotRestartableId),
+      time_source_(std::move(time_source)),
       next_sequence_number_(BeginFrameArgs::kStartingFrameNumber),
       weak_factory_(this) {
   time_source_->SetClient(this);
@@ -146,8 +161,10 @@ void BackToBackBeginFrameSource::OnTimerTick() {
 
 // DelayBasedBeginFrameSource ---------------------------------------------
 DelayBasedBeginFrameSource::DelayBasedBeginFrameSource(
-    std::unique_ptr<DelayBasedTimeSource> time_source)
-    : time_source_(std::move(time_source)),
+    std::unique_ptr<DelayBasedTimeSource> time_source,
+    uint32_t restart_id)
+    : SyntheticBeginFrameSource(restart_id),
+      time_source_(std::move(time_source)),
       next_sequence_number_(BeginFrameArgs::kStartingFrameNumber) {
   time_source_->SetClient(this);
 }
@@ -250,7 +267,7 @@ void DelayBasedBeginFrameSource::OnTimerTick() {
 // ExternalBeginFrameSource -----------------------------------------------
 ExternalBeginFrameSource::ExternalBeginFrameSource(
     ExternalBeginFrameSourceClient* client)
-    : client_(client) {
+    : BeginFrameSource(kNotRestartableId), client_(client) {
   DCHECK(client_);
 }
 

components/viz/common/frame_sinks/begin_frame_source.h

@@ -106,7 +106,17 @@ class VIZ_COMMON_EXPORT BeginFrameObserverBase : public BeginFrameObserver {
 // all BeginFrameSources *must* provide.
 class VIZ_COMMON_EXPORT BeginFrameSource {
  public:
-  BeginFrameSource();
+  // This restart_id should be used for BeginFrameSources that don't have to
+  // worry about process restart. For example, if a BeginFrameSource won't
+  // generate and forward BeginFrameArgs to another process or the process can't
+  // crash then this constant is appropriate to use.
+  static constexpr uint32_t kNotRestartableId = 0;
+
+  // If the BeginFrameSource will generate BeginFrameArgs that are forwarded to
+  // another processes *and* this process has crashed then |restart_id| should
+  // be incremented. This ensures that |source_id_| is still unique after
+  // process restart.
+  explicit BeginFrameSource(uint32_t restart_id);
   virtual ~BeginFrameSource();
 
   // Returns an identifier for this BeginFrameSource. Guaranteed unique within a
@@ -144,6 +154,8 @@ class VIZ_COMMON_EXPORT BeginFrameSource {
 // A BeginFrameSource that does nothing.
 class VIZ_COMMON_EXPORT StubBeginFrameSource : public BeginFrameSource {
  public:
+  StubBeginFrameSource();
+
   void DidFinishFrame(BeginFrameObserver* obs) override {}
   void AddObserver(BeginFrameObserver* obs) override {}
   void RemoveObserver(BeginFrameObserver* obs) override {}
@@ -153,6 +165,7 @@ class VIZ_COMMON_EXPORT StubBeginFrameSource : public BeginFrameSource {
 // A frame source which ticks itself independently.
 class VIZ_COMMON_EXPORT SyntheticBeginFrameSource : public BeginFrameSource {
  public:
+  explicit SyntheticBeginFrameSource(uint32_t restart_id);
   ~SyntheticBeginFrameSource() override;
 
   virtual void OnUpdateVSyncParameters(base::TimeTicks timebase,
@@ -201,8 +214,8 @@ class VIZ_COMMON_EXPORT DelayBasedBeginFrameSource
     : public SyntheticBeginFrameSource,
       public DelayBasedTimeSourceClient {
  public:
-  explicit DelayBasedBeginFrameSource(
-      std::unique_ptr<DelayBasedTimeSource> time_source);
+  DelayBasedBeginFrameSource(std::unique_ptr<DelayBasedTimeSource> time_source,
+                             uint32_t restart_id);
   ~DelayBasedBeginFrameSource() override;
 
   // BeginFrameSource implementation.

components/viz/common/frame_sinks/begin_frame_source_unittest.cc

@@ -349,7 +349,8 @@ class DelayBasedBeginFrameSourceTest : public ::testing::Test {
         new FakeDelayBasedTimeSource(now_src_.get(), task_runner_.get()));
     time_source->SetTimebaseAndInterval(
         base::TimeTicks(), base::TimeDelta::FromMicroseconds(10000));
-    source_.reset(new DelayBasedBeginFrameSource(std::move(time_source)));
+    source_ = std::make_unique<DelayBasedBeginFrameSource>(
+        std::move(time_source), BeginFrameSource::kNotRestartableId);
     obs_.reset(new MockBeginFrameObserver);
   }
 

components/viz/service/display_embedder/gpu_display_provider.cc

@@ -58,9 +58,11 @@ std::unique_ptr<Display> GpuDisplayProvider::CreateDisplay(
     gpu::SurfaceHandle surface_handle,
     const RendererSettings& renderer_settings,
     std::unique_ptr<BeginFrameSource>* begin_frame_source) {
+  // TODO(kylechar): Get process restart id from host process.
   auto synthetic_begin_frame_source =
       base::MakeUnique<DelayBasedBeginFrameSource>(
-          base::MakeUnique<DelayBasedTimeSource>(task_runner_.get()));
+          base::MakeUnique<DelayBasedTimeSource>(task_runner_.get()),
+          BeginFrameSource::kNotRestartableId);
 
   scoped_refptr<InProcessContextProvider> context_provider =
       new InProcessContextProvider(gpu_service_, surface_handle,

components/viz/service/frame_sinks/primary_begin_frame_source.cc

@@ -7,7 +7,7 @@
 namespace viz {
 
 PrimaryBeginFrameSource::PrimaryBeginFrameSource()
-    : begin_frame_source_(this) {}
+    : BeginFrameSource(kNotRestartableId), begin_frame_source_(this) {}
 
 PrimaryBeginFrameSource::~PrimaryBeginFrameSource() = default;
 

components/viz/test/fake_external_begin_frame_source.cc

@@ -15,7 +15,8 @@ namespace viz {
 FakeExternalBeginFrameSource::FakeExternalBeginFrameSource(
     double refresh_rate,
     bool tick_automatically)
-    : tick_automatically_(tick_automatically),
+    : BeginFrameSource(kNotRestartableId),
+      tick_automatically_(tick_automatically),
       milliseconds_per_frame_(1000.0 / refresh_rate),
       weak_ptr_factory_(this) {
   DETACH_FROM_SEQUENCE(sequence_checker_);

components/viz/test/test_layer_tree_frame_sink.cc

@@ -84,7 +84,8 @@ bool TestLayerTreeFrameSink::BindToClient(
           base::MakeUnique<DelayBasedTimeSource>(task_runner_.get()));
     } else {
       begin_frame_source_ = base::MakeUnique<DelayBasedBeginFrameSource>(
-          base::MakeUnique<DelayBasedTimeSource>(task_runner_.get()));
+          base::MakeUnique<DelayBasedTimeSource>(task_runner_.get()),
+          BeginFrameSource::kNotRestartableId);
       begin_frame_source_->SetAuthoritativeVSyncInterval(
           base::TimeDelta::FromMilliseconds(1000.f / refresh_rate_));
     }

content/browser/compositor/gpu_process_transport_factory.cc

@@ -576,7 +576,8 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
       synthetic_begin_frame_source =
           std::make_unique<viz::DelayBasedBeginFrameSource>(
               std::make_unique<viz::DelayBasedTimeSource>(
-                  compositor->task_runner().get()));
+                  compositor->task_runner().get()),
+              viz::BeginFrameSource::kNotRestartableId);
       begin_frame_source = synthetic_begin_frame_source.get();
     }
   } else {

content/browser/compositor/reflector_impl_unittest.cc

@@ -141,9 +141,10 @@ class ReflectorImplTest : public testing::Test {
         std::make_unique<NoTransportImageTransportFactory>());
     task_runner_ = message_loop_->task_runner();
     compositor_task_runner_ = new FakeTaskRunner();
-    begin_frame_source_.reset(new viz::DelayBasedBeginFrameSource(
+    begin_frame_source_ = std::make_unique<viz::DelayBasedBeginFrameSource>(
         std::make_unique<viz::DelayBasedTimeSource>(
-            compositor_task_runner_.get())));
+            compositor_task_runner_.get()),
+        viz::BeginFrameSource::kNotRestartableId);
     compositor_.reset(new ui::Compositor(
         context_factory_private->AllocateFrameSinkId(), context_factory,
         context_factory_private, compositor_task_runner_.get(),

content/browser/compositor/software_browser_compositor_output_surface_unittest.cc

@@ -66,7 +66,8 @@ class SoftwareBrowserCompositorOutputSurfaceTest : public testing::Test {
  public:
   SoftwareBrowserCompositorOutputSurfaceTest()
       : begin_frame_source_(std::make_unique<viz::DelayBasedTimeSource>(
-            message_loop_.task_runner().get())) {}
+                                message_loop_.task_runner().get()),
+                            viz::BeginFrameSource::kNotRestartableId) {}
   ~SoftwareBrowserCompositorOutputSurfaceTest() override = default;
 
   void SetUp() override;

content/renderer/gpu/compositor_external_begin_frame_source.cc

@@ -14,7 +14,8 @@ CompositorExternalBeginFrameSource::CompositorExternalBeginFrameSource(
     CompositorForwardingMessageFilter* filter,
     IPC::SyncMessageFilter* sync_message_filter,
     int routing_id)
-    : external_begin_frame_source_(this),
+    : BeginFrameSource(kNotRestartableId),
+      external_begin_frame_source_(this),
       begin_frame_source_filter_(filter),
       message_sender_(sync_message_filter),
       routing_id_(routing_id) {

ui/android/window_android.cc

@@ -27,7 +27,8 @@ using base::android::ScopedJavaLocalRef;
 class WindowAndroid::WindowBeginFrameSource : public viz::BeginFrameSource {
  public:
   explicit WindowBeginFrameSource(WindowAndroid* window)
-      : window_(window),
+      : BeginFrameSource(kNotRestartableId),
+        window_(window),
         observers_(
             base::ObserverList<viz::BeginFrameObserver>::NOTIFY_EXISTING_ONLY),
         observer_count_(0),

ui/compositor/test/in_process_context_factory.cc

@@ -254,7 +254,7 @@ void InProcessContextFactory::CreateLayerTreeFrameSink(
         base::TimeDelta::FromMicroseconds(base::Time::kMicrosecondsPerSecond /
                                           refresh_rate_));
     begin_frame_source = std::make_unique<viz::DelayBasedBeginFrameSource>(
-        std::move(time_source));
+        std::move(time_source), viz::BeginFrameSource::kNotRestartableId);
   }
   auto scheduler = std::make_unique<viz::DisplayScheduler>(
       begin_frame_source.get(), compositor->task_runner().get(),