Rename SurfaceIdAllocator to LocalSurfaceIdAllocator

Rename SurfaceIdAllocator to LocalSurfaceIdAllocator

BUG=693566
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2708153005
Cr-Commit-Position: refs/heads/master@{#452626}
Committed: https://chromium.googlesource.com/chromium/src/+/1a9aa94c031851e8e19eac0ac30e2ae25cc61136

[xing.xu, 2017-02-22 08:33:41]

Description was changed from

==========
Rename SurfaceIdAllocator to LocalSurfaceIdAllocator

BUG=693566
==========

to

==========
Rename SurfaceIdAllocator to LocalSurfaceIdAllocator

BUG=693566
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[xing.xu, 2017-02-22 08:33:43]

Description was changed from

==========
Rename SurfaceIdAllocator to LocalSurfaceIdAllocator

BUG=693566
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Rename SurfaceIdAllocator to LocalSurfaceIdAllocator

BUG=693566
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation
==========

[xing.xu, 2017-02-22 08:34:31]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[xing.xu, 2017-02-22 08:34:39]

Description was changed from

==========
Rename SurfaceIdAllocator to LocalSurfaceIdAllocator

BUG=693566
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Rename SurfaceIdAllocator to LocalSurfaceIdAllocator

BUG=693566
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation
==========

[xing.xu, 2017-02-22 08:34:39]

xing.xu@intel.com changed reviewers:
+ boliu@chromium.org, ccameron@chromium.org, fsamuel@chromium.org,
junov@chromium.org, msw@chromium.org, reveman@chromium.org

[commit-bot: I haz the power, 2017-02-22 08:35:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 10:47:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 10:47:56]

Dry run: This issue passed the CQ dry run.

[xing.xu, 2017-02-22 10:52:59]

PTAL.

[Fady Samuel, 2017-02-22 12:42:58]

lgtm + nits.

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/direct_composit...
File cc/surfaces/direct_compositor_frame_sink.h (left):

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/direct_composit...
cc/surfaces/direct_compositor_frame_sink.h:34: // this class.
nit: move this to the previous line.

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/local_surface_i...
File cc/surfaces/local_surface_id_allocator.h (right):

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/local_surface_i...
cc/surfaces/local_surface_id_allocator.h:16: // This is a helper class for
generating surface IDs within a specified
nit: Update the comment.

This is a helper class for generating local surface IDs for a single FrameSink.

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/surface_id.h
File cc/surfaces/surface_id.h (right):

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/surface_id.h#ne...
cc/surfaces/surface_id.h:71: // See LocalSurfaceIdAllocator::GenerateId.
This comment isn't particularly useful. Please delete.

[xing.xu, 2017-02-22 13:31:47]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-22 13:32:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xing.xu, 2017-02-22 13:33:29]

Done, thanks Fady!

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/direct_composit...
File cc/surfaces/direct_compositor_frame_sink.h (left):

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/direct_composit...
cc/surfaces/direct_compositor_frame_sink.h:34: // this class.
On 2017/02/22 12:42:57, Fady Samuel wrote:
> nit: move this to the previous line.

Acknowledged.

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/local_surface_i...
File cc/surfaces/local_surface_id_allocator.h (right):

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/local_surface_i...
cc/surfaces/local_surface_id_allocator.h:16: // This is a helper class for
generating surface IDs within a specified
On 2017/02/22 12:42:57, Fady Samuel wrote:
> nit: Update the comment.
> 
> This is a helper class for generating local surface IDs for a single
FrameSink.

Acknowledged.

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/surface_id.h
File cc/surfaces/surface_id.h (right):

https://codereview.chromium.org/2708153005/diff/1/cc/surfaces/surface_id.h#ne...
cc/surfaces/surface_id.h:71: // See LocalSurfaceIdAllocator::GenerateId.
On 2017/02/22 12:42:58, Fady Samuel wrote:
> This comment isn't particularly useful. Please delete.

Acknowledged.

[commit-bot: I haz the power, 2017-02-22 14:33:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 14:33:06]

Dry run: Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[xing.xu, 2017-02-22 15:27:45]

The CQ bit was checked by xing.xu@intel.com

[xing.xu, 2017-02-22 15:27:45]

The patchset sent to the CQ was uploaded after l-g-t-m from fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/2708153005/#ps20001
(title: "Refine comments")

[commit-bot: I haz the power, 2017-02-22 15:28:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xing.xu, 2017-02-22 15:28:22]

The CQ bit was unchecked by xing.xu@intel.com

[xing.xu, 2017-02-22 15:28:49]

The CQ bit was checked by xing.xu@intel.com

[commit-bot: I haz the power, 2017-02-22 15:29:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-22 15:35:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-22 15:35:53]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Fady Samuel, 2017-02-22 15:45:00]

Another minor nit! Thanks

https://codereview.chromium.org/2708153005/diff/20001/cc/surfaces/local_surfa...
File cc/surfaces/local_surface_id_allocator.h (right):

https://codereview.chromium.org/2708153005/diff/20001/cc/surfaces/local_surfa...
cc/surfaces/local_surface_id_allocator.h:17: // FrameSink within a specified
namespace. This is not threadsafe, to use from
Please remove "within a specified namespace".

[boliu, 2017-02-22 17:09:15]

https://codereview.chromium.org/2708153005/diff/20001/content/browser/composi...
File content/browser/compositor/image_transport_factory.h (right):

https://codereview.chromium.org/2708153005/diff/20001/content/browser/composi...
content/browser/compositor/image_transport_factory.h:13: #include
"cc/surfaces/local_surface_id_allocator.h"
can remove this include?

https://codereview.chromium.org/2708153005/diff/20001/content/browser/composi...
File content/browser/compositor/surface_utils.cc (right):

https://codereview.chromium.org/2708153005/diff/20001/content/browser/composi...
content/browser/compositor/surface_utils.cc:13: #include
"cc/surfaces/local_surface_id_allocator.h"
ditto, remove this instead

https://codereview.chromium.org/2708153005/diff/20001/content/browser/rendere...
File content/browser/renderer_host/delegated_frame_host.h (right):

https://codereview.chromium.org/2708153005/diff/20001/content/browser/rendere...
content/browser/renderer_host/delegated_frame_host.h:290:
std::unique_ptr<cc::LocalSurfaceIdAllocator> id_allocator_;
should forward declare allocator here, and then include the allocator in the cc
file

https://codereview.chromium.org/2708153005/diff/20001/content/browser/rendere...
File content/browser/renderer_host/offscreen_canvas_surface_impl.h (right):

https://codereview.chromium.org/2708153005/diff/20001/content/browser/rendere...
content/browser/renderer_host/offscreen_canvas_surface_impl.h:9: #include
"cc/surfaces/local_surface_id_allocator.h"
remove instead

https://codereview.chromium.org/2708153005/diff/20001/content/browser/rendere...
File content/browser/renderer_host/offscreen_canvas_surface_manager_unittest.cc
(right):

https://codereview.chromium.org/2708153005/diff/20001/content/browser/rendere...
content/browser/renderer_host/offscreen_canvas_surface_manager_unittest.cc:99:
cc::LocalSurfaceIdAllocator local_surface_id_allocator;
remove

https://codereview.chromium.org/2708153005/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_input_event_router.cc
(right):

https://codereview.chromium.org/2708153005/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_input_event_router.cc:12:
#include "cc/surfaces/local_surface_id_allocator.h"
remove

https://codereview.chromium.org/2708153005/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/2708153005/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_android.cc:35: #include
"cc/surfaces/local_surface_id_allocator.h"
remove

[xing.xu, 2017-02-23 02:18:25]

The CQ bit was checked by xing.xu@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 02:19:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 03:35:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 03:35:51]

Dry run: Try jobs failed on following builders:
  linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)

[xing.xu, 2017-02-23 05:14:58]

Done, thanks!

[boliu, 2017-02-23 05:48:03]

android_webview and content lgtm

[xing.xu, 2017-02-23 06:43:45]

The CQ bit was checked by xing.xu@intel.com

[xing.xu, 2017-02-23 06:43:46]

The patchset sent to the CQ was uploaded after l-g-t-m from fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/2708153005/#ps40001
(title: "Delete useless include")

[commit-bot: I haz the power, 2017-02-23 06:43:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 06:51:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 06:51:20]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[xing.xu, 2017-02-23 11:09:12]

The CQ bit was checked by xing.xu@intel.com

[commit-bot: I haz the power, 2017-02-23 11:09:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 11:16:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 11:16:51]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[xing.xu, 2017-02-23 13:42:17]

The CQ bit was checked by xing.xu@intel.com

[commit-bot: I haz the power, 2017-02-23 13:42:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 13:49:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 13:49:32]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[xing.xu, 2017-02-23 14:45:17]

xing.xu@intel.com changed reviewers:
+ jbauman@chromium.org

[xing.xu, 2017-02-23 14:52:08]

Thanks.Files still needs review: 
** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    cc/surfaces/BUILD.gn
    cc/surfaces/direct_compositor_frame_sink.cc
    cc/surfaces/direct_compositor_frame_sink.h
    cc/surfaces/direct_compositor_frame_sink_unittest.cc
    cc/surfaces/display_unittest.cc
    cc/surfaces/local_surface_id_allocator.cc
    cc/surfaces/local_surface_id_allocator.h
    cc/surfaces/surface.cc
    cc/surfaces/surface_aggregator_unittest.cc
    cc/surfaces/surface_hittest_unittest.cc
    cc/surfaces/surface_id.h
    cc/surfaces/surface_id_allocator.cc
    cc/surfaces/surface_id_allocator.h
    cc/surfaces/surface_manager.cc
    cc/surfaces/surface_unittest.cc
    cc/surfaces/surfaces_pixeltest.cc
    cc/test/test_compositor_frame_sink.cc
    cc/test/test_compositor_frame_sink.h
    components/exo/surface.cc
    components/exo/surface.h
    services/ui/ws/frame_generator.h
   
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
   
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.h
    ui/aura/mus/DEPS
    ui/compositor/compositor.cc
    ui/compositor/compositor_unittest.cc
    ui/compositor/test/in_process_context_factory.cc

BTW, try job prompts presubmit warnings:  The src directory requires source
formatting. Please run git cl format .
This is because in file
content/browser/renderer_host/offscreen_canvas_surface_manager_unittest.cc, git
cl format will move 
#include "content/browser/renderer_host/offscreen_canvas_surface_manager.h" to
the first line. I think this can be ignored.

[Fady Samuel, 2017-02-23 15:42:19]

On 2017/02/23 14:52:08, xing.xu wrote:
> Thanks.Files still needs review: 
> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for these files:
>     cc/surfaces/BUILD.gn
>     cc/surfaces/direct_compositor_frame_sink.cc
>     cc/surfaces/direct_compositor_frame_sink.h
>     cc/surfaces/direct_compositor_frame_sink_unittest.cc
>     cc/surfaces/display_unittest.cc
>     cc/surfaces/local_surface_id_allocator.cc
>     cc/surfaces/local_surface_id_allocator.h
>     cc/surfaces/surface.cc
>     cc/surfaces/surface_aggregator_unittest.cc
>     cc/surfaces/surface_hittest_unittest.cc
>     cc/surfaces/surface_id.h
>     cc/surfaces/surface_id_allocator.cc
>     cc/surfaces/surface_id_allocator.h
>     cc/surfaces/surface_manager.cc
>     cc/surfaces/surface_unittest.cc
>     cc/surfaces/surfaces_pixeltest.cc
>     cc/test/test_compositor_frame_sink.cc
>     cc/test/test_compositor_frame_sink.h
>     components/exo/surface.cc
>     components/exo/surface.h
>     services/ui/ws/frame_generator.h
>    
>
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
>    
>
third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.h
>     ui/aura/mus/DEPS
>     ui/compositor/compositor.cc
>     ui/compositor/compositor_unittest.cc
>     ui/compositor/test/in_process_context_factory.cc
> 
> BTW, try job prompts presubmit warnings:  The src directory requires source
> formatting. Please run git cl format .
> This is because in file
> content/browser/renderer_host/offscreen_canvas_surface_manager_unittest.cc,
git
> cl format will move 
> #include "content/browser/renderer_host/offscreen_canvas_surface_manager.h" to
> the first line. I think this can be ignored.

Please run git cl format?

Please add vmpstr@ for cc/ changes, reveman@ for exo changes, sky@ for
services/ui, and jam@ for everything else (content/).

[Fady Samuel, 2017-02-23 15:58:59]

fsamuel@chromium.org changed reviewers:
+ jam@chromium.org, sky@chromium.org, vmpstr@chromium.org
- ccameron@chromium.org, jbauman@chromium.org, msw@chromium.org

[Fady Samuel, 2017-02-23 15:59:00]

PTAL

vmpstr@ for cc
reveman@ for exo changes
sky@ for services/ui, and
jam@ for everything else (content/).
junov@ for offscreen canvas

[reveman, 2017-02-23 17:10:09]

components/exo and cc lgtm

[Justin Novosad, 2017-02-23 19:06:27]

third_party/WebKit/Source/platform/graphics/  lgtm

[sky, 2017-02-23 20:33:21]

LGTM

[jam, 2017-02-23 20:36:18]

lgtm

[Fady Samuel, 2017-02-23 20:55:43]

The CQ bit was checked by fsamuel@chromium.org

[commit-bot: I haz the power, 2017-02-23 20:56:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 21:04:37]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487883343516850,
"parent_rev": "930324f05148191a536e4500d9b492c059a4771e", "commit_rev":
"1a9aa94c031851e8e19eac0ac30e2ae25cc61136"}

[commit-bot: I haz the power, 2017-02-23 21:05:16]

Description was changed from

==========
Rename SurfaceIdAllocator to LocalSurfaceIdAllocator

BUG=693566
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Rename SurfaceIdAllocator to LocalSurfaceIdAllocator

BUG=693566
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2708153005
Cr-Commit-Position: refs/heads/master@{#452626}
Committed:
https://chromium.googlesource.com/chromium/src/+/1a9aa94c031851e8e19eac0ac30e...
==========

[commit-bot: I haz the power, 2017-02-23 21:05:19]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/1a9aa94c031851e8e19eac0ac30e...

[xing.xu, 2017-02-24 03:11:54]

About git cl format warnings, submit a bug here:
https://bugs.chromium.org/p/chromium/issues/detail?id=695744