Delete the logPrivate APIs

Delete the logPrivate APIs

This API is not used and can be replaced by the newer
feedbackPrivate.getSystemInformation APIs.

BUG=710158
Review-Url: https://codereview.chromium.org/2934163002
Cr-Commit-Position: refs/heads/master@{#479855}
Committed: https://chromium.googlesource.com/chromium/src/+/4657c6480527b4b1ea6cb21d4d666ea92652f04d

[afakhry, 2017-06-13 22:23:49]

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

[commit-bot: I haz the power, 2017-06-13 22:24:22]

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-06-14 00:01:23]

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

[commit-bot: I haz the power, 2017-06-14 00:01:24]

Dry run: This issue passed the CQ dry run.

[afakhry, 2017-06-14 15:56:02]

afakhry@chromium.org changed reviewers:
+ tbarzic@chromium.org, thestig@chromium.org

[afakhry, 2017-06-14 15:56:02]

tbarzic@chromium.org: for .*log_private.*

thestig@chromium.org: for the rest.

Thanks!

[tbarzic, 2017-06-15 01:46:05]

nice :)
lgtm

https://codereview.chromium.org/2934163002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/log_private/log_private_api.h (left):

https://codereview.chromium.org/2934163002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/log_private/log_private_api.h:156:
LOGPRIVATE_STARTEVENTRECODER);
LOGPRIVATE_ enums to DELETED_LOGPRIVATE_
but it might be clearer to do it in a follow up :)

[Lei Zhang, 2017-06-15 16:51:20]

lgtm

[afakhry, 2017-06-15 17:00:00]

https://codereview.chromium.org/2934163002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/log_private/log_private_api.h (left):

https://codereview.chromium.org/2934163002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/log_private/log_private_api.h:156:
LOGPRIVATE_STARTEVENTRECODER);
On 2017/06/15 01:46:05, tbarzic wrote:
> LOGPRIVATE_ enums to DELETED_LOGPRIVATE_
> but it might be clearer to do it in a follow up :)

Can we do it in this CL? That way if we need to revert for some reason, we
revert everything together.

Also, I have a question. Should we remove those instances too?:

-
https://cs.chromium.org/chromium/src/chrome/common/extensions/docs/server2/te...


-
https://cs.chromium.org/chromium/src/chrome/common/extensions/docs/server2/kn...

-
https://cs.chromium.org/chromium/src/chrome/common/extensions/api/_permission...

-
https://cs.chromium.org/chromium/src/chrome/common/extensions/api/_api_featur...

[Lei Zhang, 2017-06-15 17:20:15]

https://codereview.chromium.org/2934163002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/log_private/log_private_api.h (left):

https://codereview.chromium.org/2934163002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/log_private/log_private_api.h:156:
LOGPRIVATE_STARTEVENTRECODER);
On 2017/06/15 17:00:00, afakhry wrote:
> On 2017/06/15 01:46:05, tbarzic wrote:
> > LOGPRIVATE_ enums to DELETED_LOGPRIVATE_
> > but it might be clearer to do it in a follow up :)
> 
> Can we do it in this CL? That way if we need to revert for some reason, we
> revert everything together.

Sure.

> Also, I have a question. Should we remove those instances too?:

Not sure. You can always ask Devlin.

[afakhry, 2017-06-15 17:39:01]

afakhry@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[afakhry, 2017-06-15 17:39:02]

Devlin, can you please take a look at my question below? Thanks!

https://codereview.chromium.org/2934163002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/log_private/log_private_api.h (left):

https://codereview.chromium.org/2934163002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/log_private/log_private_api.h:156:
LOGPRIVATE_STARTEVENTRECODER);
On 2017/06/15 17:20:15, Lei Zhang wrote:
> On 2017/06/15 17:00:00, afakhry wrote:
> > On 2017/06/15 01:46:05, tbarzic wrote:
> > > LOGPRIVATE_ enums to DELETED_LOGPRIVATE_
> > > but it might be clearer to do it in a follow up :)
> > 
> > Can we do it in this CL? That way if we need to revert for some reason, we
> > revert everything together.
> 
> Sure.
> 
> > Also, I have a question. Should we remove those instances too?:
> 
> Not sure. You can always ask Devlin.

+Devlin for my above question.

[Devlin, 2017-06-15 17:54:31]

Also, in addition to the function histogram entries tbarzic@ mentioned, please
rename (but do not remove!):
- The permission in api_permissions.h
- The on_captured_events event histogram entry in
extensions/browser/extension_event_histogram_value.h

https://codereview.chromium.org/2934163002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/log_private/log_private_api.h (left):

https://codereview.chromium.org/2934163002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/log_private/log_private_api.h:156:
LOGPRIVATE_STARTEVENTRECODER);
On 2017/06/15 17:00:00, afakhry wrote:
> On 2017/06/15 01:46:05, tbarzic wrote:
> > LOGPRIVATE_ enums to DELETED_LOGPRIVATE_
> > but it might be clearer to do it in a follow up :)
> 
> Can we do it in this CL? That way if we need to revert for some reason, we
> revert everything together.
> 
> Also, I have a question. Should we remove those instances too?:
> 
> -
>
https://cs.chromium.org/chromium/src/chrome/common/extensions/docs/server2/te...
> 
> 
> -
>
https://cs.chromium.org/chromium/src/chrome/common/extensions/docs/server2/kn...
> 
> -
>
https://cs.chromium.org/chromium/src/chrome/common/extensions/api/_permission...
> 
> -
>
https://cs.chromium.org/chromium/src/chrome/common/extensions/api/_api_featur...

Yep, I think these can all be deleted.

[afakhry, 2017-06-15 19:10:31]

afakhry@chromium.org changed reviewers:
+ isherman@chromium.org

[afakhry, 2017-06-15 19:10:31]

Thanks, Devlin! Done. Please take a look.

 extensions/browser/extension_event_histogram_value.h	View	1	1 chunk	+1 line, -1
line	0 comments	Download
M	 extensions/browser/extension_function_histogram_value.h

[afakhry, 2017-06-15 19:11:29]

+isherman@chromium.org for:
    extensions/browser/extension_event_histogram_value.h
    extensions/browser/extension_function_histogram_value.h

[Devlin, 2017-06-15 19:17:37]

sorry for the churn in docs/ - I forgot this might mean rolling a new version of
the server.

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/app.yaml (right):

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/app.yaml:2: version: 3-53-0
Hmm... actually, I'm going to backpedal and say we should hold off on the docs/
changes.  These can be tricky, so we should do them in a separate CL.

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
File chrome/common/extensions/permissions/chrome_api_permissions.cc (right):

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
chrome/common/extensions/permissions/chrome_api_permissions.cc:133:
{APIPermission::kDeletedLogPrivate, "logPrivate",
This can be completely removed.

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
File chrome/common/extensions/permissions/permission_set_unittest.cc (right):

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
chrome/common/extensions/permissions/permission_set_unittest.cc:758:
skip.insert(APIPermission::kDeletedLogPrivate);
ditto

https://codereview.chromium.org/2934163002/diff/20001/extensions/common/permi...
File extensions/common/permissions/api_permission.h (right):

https://codereview.chromium.org/2934163002/diff/20001/extensions/common/permi...
extensions/common/permissions/api_permission.h:140: kDeletedLogPrivate,
nit: prefer kDeleted_LogPrivate

https://codereview.chromium.org/2934163002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/enums.xml (right):

https://codereview.chromium.org/2934163002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/enums.xml:13069: +  <int value="97"
label="kDeletedLogPrivate"/>
ditto - kDeleted_LogPrivate

[afakhry, 2017-06-15 19:26:50]

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
File chrome/common/extensions/docs/server2/app.yaml (right):

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
chrome/common/extensions/docs/server2/app.yaml:2: version: 3-53-0
On 2017/06/15 19:17:37, Devlin wrote:
> Hmm... actually, I'm going to backpedal and say we should hold off on the
docs/
> changes.  These can be tricky, so we should do them in a separate CL.

Done.

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
File chrome/common/extensions/permissions/chrome_api_permissions.cc (right):

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
chrome/common/extensions/permissions/chrome_api_permissions.cc:133:
{APIPermission::kDeletedLogPrivate, "logPrivate",
On 2017/06/15 19:17:37, Devlin wrote:
> This can be completely removed.

Done.

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
File chrome/common/extensions/permissions/permission_set_unittest.cc (right):

https://codereview.chromium.org/2934163002/diff/20001/chrome/common/extension...
chrome/common/extensions/permissions/permission_set_unittest.cc:758:
skip.insert(APIPermission::kDeletedLogPrivate);
On 2017/06/15 19:17:37, Devlin wrote:
> ditto

Done.

https://codereview.chromium.org/2934163002/diff/20001/extensions/common/permi...
File extensions/common/permissions/api_permission.h (right):

https://codereview.chromium.org/2934163002/diff/20001/extensions/common/permi...
extensions/common/permissions/api_permission.h:140: kDeletedLogPrivate,
On 2017/06/15 19:17:37, Devlin wrote:
> nit: prefer kDeleted_LogPrivate

Done.

https://codereview.chromium.org/2934163002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/enums.xml (right):

https://codereview.chromium.org/2934163002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/enums.xml:13069: +  <int value="97"
label="kDeletedLogPrivate"/>
On 2017/06/15 19:17:37, Devlin wrote:
> ditto - kDeleted_LogPrivate

Done.

[Ilya Sherman, 2017-06-15 19:36:20]

enum changes lgtm

[Devlin, 2017-06-15 20:02:20]

lgtm; thanks for the cleanup!

[afakhry, 2017-06-15 20:19:56]

The CQ bit was checked by afakhry@chromium.org

[afakhry, 2017-06-15 20:19:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, tbarzic@chromium.org
Link to the patchset: https://codereview.chromium.org/2934163002/#ps40001
(title: "Roll back docs changes")

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

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-06-15 21:58:38]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1497557996878270,
"parent_rev": "14085ad54d50252bcadd1b1d7dc4a0bca2c9c5b3", "commit_rev":
"4657c6480527b4b1ea6cb21d4d666ea92652f04d"}

[commit-bot: I haz the power, 2017-06-15 21:58:52]

Description was changed from

==========
Delete the logPrivate APIs

This API is not used and can be replaced by the newer
feedbackPrivate.getSystemInformation APIs.

BUG=710158
==========

to

==========
Delete the logPrivate APIs

This API is not used and can be replaced by the newer
feedbackPrivate.getSystemInformation APIs.

BUG=710158

Review-Url: https://codereview.chromium.org/2934163002
Cr-Commit-Position: refs/heads/master@{#479855}
Committed:
https://chromium.googlesource.com/chromium/src/+/4657c6480527b4b1ea6cb21d4d66...
==========

[commit-bot: I haz the power, 2017-06-15 21:58:54]

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