GCM: Make InstanceIDHandler implementations use protected inheritance

GCM: Make InstanceIDHandler implementations use protected inheritance

This avoids accidental layering violations, by enforcing use of
GCMDriver::GetInstanceIDHandlerInternal (which is commented as "These
methods must only be used by the InstanceID system") to get access to
InstanceIDHandlers.

BUG=none

Committed: https://crrev.com/8a2e23085bff76b06f35f83e38260a1ce39d6e58
Cr-Commit-Position: refs/heads/master@{#441425}

[johnme, 2017-01-04 17:15:34]

johnme@chromium.org changed reviewers:
+ peter@chromium.org

[Peter Beverloo, 2017-01-04 17:47:38]

lgtm % comment

Despite that, I'm not a fan of these changes. It's not exactly a foot gun, use
of protected inheritance is very limited in Chromium (I count 11 instances) and
it goes against the guidance from the style guide ("all inheritance should be
public"). Let's stop after this?

https://codereview.chromium.org/2610933002/diff/1/components/gcm_driver/insta...
File components/gcm_driver/instance_id/fake_gcm_driver_for_instance_id.h
(right):

https://codereview.chromium.org/2610933002/diff/1/components/gcm_driver/insta...
components/gcm_driver/instance_id/fake_gcm_driver_for_instance_id.h:44: //
InstanceIDHandler implementation:
This is now inconsistent with line 30.

[johnme, 2017-01-04 18:12:38]

> It's not exactly a foot gun

I was reviewing a patch just today accidentally calling
FakeGCMDriverForInstanceID::AddInstanceIDData from PushMessagingBrowserTest
because it was the minimal change needed to get the test passing, even though it
results in a partially invalid and hence somewhat unpredictable subscription
(hence the benefit of encapsulating things, so that the pit of success is to
call the proper method that creates a valid subscription).

https://codereview.chromium.org/2610933002/diff/1/components/gcm_driver/insta...
File components/gcm_driver/instance_id/fake_gcm_driver_for_instance_id.h
(right):

https://codereview.chromium.org/2610933002/diff/1/components/gcm_driver/insta...
components/gcm_driver/instance_id/fake_gcm_driver_for_instance_id.h:44: //
InstanceIDHandler implementation:
On 2017/01/04 17:47:38, Peter Beverloo wrote:
> This is now inconsistent with line 30.

That's because these are implementations of pure virtual methods, whereas line
30 is an override of a method already implemented by FakeGCMDriver. But changed
it to overrides because it's not important.

[johnme, 2017-01-04 18:12:42]

The CQ bit was checked by johnme@chromium.org

[johnme, 2017-01-04 18:12:42]

The patchset sent to the CQ was uploaded after l-g-t-m from peter@chromium.org
Link to the patchset: https://codereview.chromium.org/2610933002/#ps20001
(title: "s/implementation/overrides/")

[commit-bot: I haz the power, 2017-01-04 18:12: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-01-04 19:11:17]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483553562110100,
"parent_rev": "4bc51990d1cea583d9ccdd6e00ea6b572739a682", "commit_rev":
"52a970a6820a60dd8bca5733f4394f9de786156e"}

[commit-bot: I haz the power, 2017-01-04 19:11:48]

Description was changed from

==========
GCM: Make InstanceIDHandler implementations use protected inheritance

This avoids accidental layering violations, by enforcing use of
GCMDriver::GetInstanceIDHandlerInternal (which is commented as "These
methods must only be used by the InstanceID system") to get access to
InstanceIDHandlers.

BUG=none
==========

to

==========
GCM: Make InstanceIDHandler implementations use protected inheritance

This avoids accidental layering violations, by enforcing use of
GCMDriver::GetInstanceIDHandlerInternal (which is commented as "These
methods must only be used by the InstanceID system") to get access to
InstanceIDHandlers.

BUG=none

Review-Url: https://codereview.chromium.org/2610933002
==========

[commit-bot: I haz the power, 2017-01-04 19:11:49]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2017-01-04 19:15:19]

Description was changed from

==========
GCM: Make InstanceIDHandler implementations use protected inheritance

This avoids accidental layering violations, by enforcing use of
GCMDriver::GetInstanceIDHandlerInternal (which is commented as "These
methods must only be used by the InstanceID system") to get access to
InstanceIDHandlers.

BUG=none

Review-Url: https://codereview.chromium.org/2610933002
==========

to

==========
GCM: Make InstanceIDHandler implementations use protected inheritance

This avoids accidental layering violations, by enforcing use of
GCMDriver::GetInstanceIDHandlerInternal (which is commented as "These
methods must only be used by the InstanceID system") to get access to
InstanceIDHandlers.

BUG=none

Committed: https://crrev.com/8a2e23085bff76b06f35f83e38260a1ce39d6e58
Cr-Commit-Position: refs/heads/master@{#441425}
==========

[commit-bot: I haz the power, 2017-01-04 19:15:21]

Patchset 2 (id:??) landed as
https://crrev.com/8a2e23085bff76b06f35f83e38260a1ce39d6e58
Cr-Commit-Position: refs/heads/master@{#441425}