Make Satisfy/Require callbacks pass Surface id and sequence by reference

Make Satisfy/Require callbacks pass Surface id and sequence by reference

BUG=None
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/92e65345e6377de1ac9c2f3d783dde6f7b1aa65b
Cr-Commit-Position: refs/heads/master@{#403187}

[xlai (Olivia), 2016-06-28 15:51:45]

Description was changed from

==========
Make SurfaceId, SurfaceSequence pass by reference on callbacks

BUG=None
==========

to

==========
Make SurfaceId, SurfaceSequence pass by reference on callbacks

BUG=None
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[xlai (Olivia), 2016-06-28 15:52:14]

The CQ bit was checked by xlai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-28 15:52:35]

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

[xlai (Olivia), 2016-06-28 15:55:09]

Description was changed from

==========
Make SurfaceId, SurfaceSequence pass by reference on callbacks

BUG=None
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Make Satisfy/Require callbacks pass Surface id and sequence by reference

BUG=None
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-06-28 16:06:03]

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

[commit-bot: I haz the power, 2016-06-28 16:06:03]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[xlai (Olivia), 2016-06-28 16:11:14]

The CQ bit was checked by xlai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-28 16:11:56]

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, 2016-06-28 16:30:54]

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

[commit-bot: I haz the power, 2016-06-28 16:30:55]

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

[xlai (Olivia), 2016-06-28 16:43:34]

The CQ bit was checked by xlai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-28 16:44: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, 2016-06-28 18:12:40]

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

[commit-bot: I haz the power, 2016-06-28 18:12:42]

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

[xlai (Olivia), 2016-06-28 18:53:00]

The CQ bit was checked by xlai@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-28 18:53:47]

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

[xlai (Olivia), 2016-06-28 19:18:32]

xlai@chromium.org changed reviewers:
+ ben@chromium.org

[commit-bot: I haz the power, 2016-06-28 20:40:58]

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

[commit-bot: I haz the power, 2016-06-28 20:40:59]

Dry run: This issue passed the CQ dry run.

[xlai (Olivia), 2016-06-28 21:16:19]

This patch does some refactoring on functions that pass SurfaceId and
SurfaceSequence in parameters. 

Currently, there is an inconsistent mix of usage of "pass by value" and "pass by
const reference" for these two classes. As it's a C++ convention to pass
untrivial class by const&, thereby fixing it. Also did some other minor style
amendments in this CL.

https://codereview.chromium.org/2102123002/diff/60001/cc/surfaces/surface_fac...
File cc/surfaces/surface_factory.h (right):

https://codereview.chromium.org/2102123002/diff/60001/cc/surfaces/surface_fac...
cc/surfaces/surface_factory.h:88: void did_destroy_surface_manager() { manager_
= nullptr; }
The previous function name uses Blink style instead of Chromium style.

[Ben Goodger (Google), 2016-06-28 21:27:34]

ben@chromium.org changed reviewers:
+ rjkroege@chromium.org

[Ben Goodger (Google), 2016-06-28 21:27:35]

-me +rjk

[danakj, 2016-06-28 22:13:13]

danakj@chromium.org changed reviewers:
+ danakj@chromium.org

[danakj, 2016-06-28 22:13:15]

cc LGTM

https://codereview.chromium.org/2102123002/diff/60001/cc/surfaces/surface_fac...
File cc/surfaces/surface_factory.h (right):

https://codereview.chromium.org/2102123002/diff/60001/cc/surfaces/surface_fac...
cc/surfaces/surface_factory.h:88: void did_destroy_surface_manager() { manager_
= nullptr; }
On 2016/06/28 21:16:19, xlai (Olivia) wrote:
> The previous function name uses Blink style instead of Chromium style.

I think I prefer DidDestroySurfaceManager instead.

hacker_style() that doesn't start with set_ looks like an accessor to me. I know
the style guide loosened the times when you're allowed to use it to any "cheap"
function, but I'd prefer that still here.

https://codereview.chromium.org/2102123002/diff/60001/cc/surfaces/surface_id_...
File cc/surfaces/surface_id_allocator.h (right):

https://codereview.chromium.org/2102123002/diff/60001/cc/surfaces/surface_id_...
cc/surfaces/surface_id_allocator.h:40: void did_destroy_surface_manager() {
manager_ = nullptr; }
and here

[rjkroege, 2016-06-28 23:31:55]

mus/gpu lgtm

https://codereview.chromium.org/2102123002/diff/60001/content/browser/rendere...
File content/browser/renderer_host/offscreen_canvas_surface_impl.cc (right):

https://codereview.chromium.org/2102123002/diff/60001/content/browser/rendere...
content/browser/renderer_host/offscreen_canvas_surface_impl.cc:31:
surface_factory_->did_destroy_surface_manager();
danakj@'s comment applies here too yes?

[xlai (Olivia), 2016-06-29 14:11:23]

xlai@chromium.org changed reviewers:
+ kinuko@chromium.org, reveman@chromium.org

[xlai (Olivia), 2016-06-29 14:11:24]

This is a refactoring CL. I renamed the function to chromium style as
accordingly.

Also, add more reviewers as it happened that the refactoring affects multiple
parts:

+kinuko for reviews on third_party/WebKit/ and content/.
+reveman for reviews on components/exo/

[reveman, 2016-06-29 16:51:15]

components/exo lgtm

[kinuko, 2016-06-30 09:31:15]

content and WebKit lgtm

[xlai (Olivia), 2016-06-30 14:24:17]

The CQ bit was checked by xlai@chromium.org

[xlai (Olivia), 2016-06-30 14:24:18]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org, danakj@chromium.org, kinuko@chromium.org,
reveman@chromium.org
Link to the patchset: https://codereview.chromium.org/2102123002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-06-30 14:24:33]

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, 2016-06-30 15:59:17]

Description was changed from

==========
Make Satisfy/Require callbacks pass Surface id and sequence by reference

BUG=None
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Make Satisfy/Require callbacks pass Surface id and sequence by reference

BUG=None
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-06-30 15:59:18]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-30 15:59:31]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-30 16:01:20]

Description was changed from

==========
Make Satisfy/Require callbacks pass Surface id and sequence by reference

BUG=None
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Make Satisfy/Require callbacks pass Surface id and sequence by reference

BUG=None
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/92e65345e6377de1ac9c2f3d783dde6f7b1aa65b
Cr-Commit-Position: refs/heads/master@{#403187}
==========

[commit-bot: I haz the power, 2016-06-30 16:01:22]

Patchset 6 (id:??) landed as
https://crrev.com/92e65345e6377de1ac9c2f3d783dde6f7b1aa65b
Cr-Commit-Position: refs/heads/master@{#403187}