Make tree id sequence in LayerTreeHost thread-safe.

Make tree id sequence in LayerTreeHost thread-safe.

LayerTreeHost instances can exists in multiple threads. e.g. Aura with
--single-process. So this CL makes the sequence thread-safe.

In addition, this CL removes unused s_num_layer_tree_instances.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233047

[jamesr, 2013-11-04 19:18:01]

https://codereview.chromium.org/57713004/diff/1/cc/trees/layer_tree_host.cc
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/57713004/diff/1/cc/trees/layer_tree_host.cc#n...
cc/trees/layer_tree_host.cc:51: (void)initializeTo1;
what does this line do? I don't think this complexity is necessary. this is
rarely called, can you just use an integer and a lock?

[dshwang, 2013-11-04 19:34:47]

https://codereview.chromium.org/57713004/diff/1/cc/trees/layer_tree_host.cc
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/57713004/diff/1/cc/trees/layer_tree_host.cc#n...
cc/trees/layer_tree_host.cc:51: (void)initializeTo1;
On 2013/11/04 19:18:01, jamesr wrote:
> what does this line do? I don't think this complexity is necessary. this is
> rarely called, can you just use an integer and a lock?

Thank you for review. Done.

[jamesr, 2013-11-04 20:11:26]

lgtm

[commit-bot: I haz the power, 2013-11-04 20:56:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/57713004/90001

[commit-bot: I haz the power, 2013-11-05 01:22:59]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-11-05 07:13:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/57713004/90001

[commit-bot: I haz the power, 2013-11-05 10:00:14]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-11-05 10:47:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/57713004/90001

[commit-bot: I haz the power, 2013-11-05 14:27:55]

Failed to apply patch for cc/trees/layer_tree_host.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file cc/trees/layer_tree_host.cc
  Hunk #6 FAILED at 184.
  1 out of 6 hunks FAILED -- saving rejects to file
cc/trees/layer_tree_host.cc.rej

Patch:       cc/trees/layer_tree_host.cc
Index: cc/trees/layer_tree_host.cc
diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc
index
d93e3d5e032e80c8cf353b672338904f832a16f2..9da17f44b89e4d8d74ed3b0e9948d56e3b989f82
100644
--- a/cc/trees/layer_tree_host.cc
+++ b/cc/trees/layer_tree_host.cc
@@ -11,10 +11,12 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
+#include "base/lazy_instance.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/synchronization/lock.h"
 #include "cc/animation/animation_registrar.h"
 #include "cc/animation/layer_animation_controller.h"
 #include "cc/base/math_util.h"
@@ -41,7 +43,14 @@
 #include "ui/gfx/size_conversions.h"
 
 namespace {
-static int s_num_layer_tree_instances;
+static base::LazyInstance<base::Lock>::Leaky
+    s_next_tree_id_lock = LAZY_INSTANCE_INITIALIZER;
+
+inline int GetNextTreeId() {
+  static int s_next_tree_id = 1;
+  base::AutoLock lock(s_next_tree_id_lock.Get());
+  return s_next_tree_id++;
+}
 }
 
 namespace cc {
@@ -89,10 +98,6 @@ UIResourceRequest& UIResourceRequest::operator=(
 
 UIResourceRequest::~UIResourceRequest() {}
 
-bool LayerTreeHost::AnyLayerTreeHostInstanceExists() {
-  return s_num_layer_tree_instances > 0;
-}
-
 scoped_ptr<LayerTreeHost> LayerTreeHost::Create(
     LayerTreeHostClient* client,
     SharedBitmapManager* manager,
@@ -105,8 +110,6 @@ scoped_ptr<LayerTreeHost> LayerTreeHost::Create(
   return layer_tree_host.Pass();
 }
 
-static int s_next_tree_id = 1;
-
 LayerTreeHost::LayerTreeHost(LayerTreeHostClient* client,
                              SharedBitmapManager* manager,
                              const LayerTreeSettings& settings)
@@ -135,12 +138,11 @@ LayerTreeHost::LayerTreeHost(LayerTreeHostClient* client,
       partial_texture_update_requests_(0),
       in_paint_layer_contents_(false),
       total_frames_used_for_lcd_text_metrics_(0),
-      tree_id_(s_next_tree_id++),
+      tree_id_(GetNextTreeId()),
       next_commit_forces_redraw_(false),
       shared_bitmap_manager_(manager) {
   if (settings_.accelerated_animation_enabled)
     animation_registrar_ = AnimationRegistrar::Create();
-  s_num_layer_tree_instances++;
   rendering_stats_instrumentation_->set_record_rendering_stats(
       debug_state_.RecordRenderingStats());
 }
@@ -182,7 +184,6 @@ LayerTreeHost::~LayerTreeHost() {
     proxy_->Stop();
   }
 
-  s_num_layer_tree_instances--;
   RateLimiterMap::iterator it = rate_limiters_.begin();
   if (it != rate_limiters_.end())
     it->second->Stop();

[commit-bot: I haz the power, 2013-11-05 14:54:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/57713004/380001

[commit-bot: I haz the power, 2013-11-05 16:59:15]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-11-05 17:13:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/57713004/380001

[commit-bot: I haz the power, 2013-11-05 17:20:27]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-11-05 18:19:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dongseong.hwang@intel.com/57713004/380001

[commit-bot: I haz the power, 2013-11-05 19:11:20]

Change committed as 233047

[caseq, 2013-11-07 09:20:02]

https://codereview.chromium.org/57713004/diff/380001/cc/trees/layer_tree_host.cc
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/57713004/diff/380001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host.cc:51: base::AutoLock lock(s_next_tree_id_lock.Get());
Any reason to not use StaticAtomicSequenceNumber here?

[dshwang, 2013-11-07 09:46:02]

https://codereview.chromium.org/57713004/diff/380001/cc/trees/layer_tree_host.cc
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/57713004/diff/380001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host.cc:51: base::AutoLock lock(s_next_tree_id_lock.Get());
On 2013/11/07 09:20:03, caseq wrote:
> Any reason to not use StaticAtomicSequenceNumber here?

Hi, Patch Set 1 used AtomicSequenceNumber, but AtomicSequenceNumber is
initialized to 0, so there was additional code to set 1 to it.
It looked a bit complicated, and this is very rare called. so we decided to use
lock.

[caseq, 2013-11-07 10:05:24]

On 2013/11/07 09:46:02, dshwang wrote:
>
https://codereview.chromium.org/57713004/diff/380001/cc/trees/layer_tree_host.cc
> File cc/trees/layer_tree_host.cc (right):
> 
>
https://codereview.chromium.org/57713004/diff/380001/cc/trees/layer_tree_host...
> cc/trees/layer_tree_host.cc:51: base::AutoLock
lock(s_next_tree_id_lock.Get());
> On 2013/11/07 09:20:03, caseq wrote:
> > Any reason to not use StaticAtomicSequenceNumber here?
> 
> Hi, Patch Set 1 used AtomicSequenceNumber, but AtomicSequenceNumber is
> initialized to 0, so there was additional code to set 1 to it.
> It looked a bit complicated, and this is very rare called. so we decided to
use
> lock.

Ugh, I see, I missed the first ps, sorry! But why don't we just do GetNext() + 1
then? I think this is overall a shorter and easier to understand code. I'm
changing to this as a drive-by in https://codereview.chromium.org/60353002/, if
you mind this, please state it there.