struct UnloadedExtensionInfo -> enum UnloadedExtensionInfoReason

struct UnloadedExtensionInfo -> enum UnloadedExtensionInfoReason

We don't need to keep struct UnloadedExtensionInfo.
So removing it and use enum UnloadedExtensionInfoReason instead.

TEST=Everything runs smoothly
BUG=715144
Review-Url: https://codereview.chromium.org/2839373003
Cr-Commit-Position: refs/heads/master@{#469065}
Committed: https://chromium.googlesource.com/chromium/src/+/0deef20444dd64caa41f81b5175925d9a5bf3649

[limasdf, 2017-04-27 10:12:07]

Description was changed from

==========
ExtensionUnloadedInfo cleanup 2

BUG=
==========

to

==========
ExtensionUnloadedInfo cleanup 2

BUG=715144
==========

[limasdf, 2017-04-29 10:31:13]

Description was changed from

==========
ExtensionUnloadedInfo cleanup 2

BUG=715144
==========

to

==========
struct UnloadedExtensionInfo -> enum UnloadedExtensionInfoReason

We don't need to keep struct UnloadedExtensionInfo.
So removing it and use enum UnloadedExtensionInfoReason instead.

BUG=715144
==========

[limasdf, 2017-04-29 10:31:13]

limasdf@gmail.com changed reviewers:
+ rdevlin.cronin@chromium.org

[limasdf, 2017-04-29 10:34:10]

limasdf@gmail.com changed reviewers:
+ sky@chromium.org

[limasdf, 2017-04-29 10:44:47]

Hi, Devlin and Sky.

Do you mind taking a look?

Devlin: /extensions/*
Sky: /chrome/*

Sorry for the huge change, but this is a mechanical change except for
extension/common/extension.[h/cc].
I mentioned the thing I did in the patch description.

[limasdf, 2017-04-29 10:46:11]

Description was changed from

==========
struct UnloadedExtensionInfo -> enum UnloadedExtensionInfoReason

We don't need to keep struct UnloadedExtensionInfo.
So removing it and use enum UnloadedExtensionInfoReason instead.

BUG=715144
==========

to

==========
struct UnloadedExtensionInfo -> enum UnloadedExtensionInfoReason

We don't need to keep struct UnloadedExtensionInfo.
So removing it and use enum UnloadedExtensionInfoReason instead.

TEST=Everything runs smoothly
BUG=715144
==========

[Devlin, 2017-05-01 14:08:58]

Nice!  Thanks for cleaning this up!

https://codereview.chromium.org/2839373003/diff/100001/chrome/browser/content...
File
chrome/browser/content_settings/content_settings_internal_extension_provider.cc
(right):

https://codereview.chromium.org/2839373003/diff/100001/chrome/browser/content...
chrome/browser/content_settings/content_settings_internal_extension_provider.cc:27:
using extensions::UnloadedExtensionReason;
I know this was just copy-replace, but it looks like this isn't needed.

https://codereview.chromium.org/2839373003/diff/100001/extensions/common/exte...
File extensions/common/extension.h (right):

https://codereview.chromium.org/2839373003/diff/100001/extensions/common/exte...
extensions/common/extension.h:518: enum UnloadedExtensionReason {
We should make this an enum class; otherwise the values are defined at the scope
of the extensions namespace (e.g., extensions::REASON_UNDEFINED would work -
which is a little too vague).

Two options:
1) Just make this an enum class in this file; everything should still work with
no other changes (because you've specified all the enums with the more targeted
scoping already in the code).
2) Make this an enum class and remove the REASON_ prefix from the values.  This
would be a *bit* cleaner, because UnloadedExtensionReason::REASON_FOO is a
little redundant compared to UnloadedExtensionReason::FOO.  If you have a
find-replace script that works well, this shouldn't be too hard
(s/UnloadedExtensionReason::REASON_/UnloadedExtensionReason::), but if there's
any trouble, then don't worry about it - it's a very small detail. :)

[limasdf, 2017-05-02 06:36:22]

Patchset #8 (id:140001) has been deleted

[limasdf, 2017-05-02 06:56:09]

Patchset #9 (id:180001) has been deleted

[limasdf, 2017-05-02 07:50:17]

Patchset #9 (id:200001) has been deleted

[limasdf, 2017-05-02 07:52:52]

Thanks for the review.
I have checked every single file and they look good to me.
PTAL.

https://codereview.chromium.org/2839373003/diff/100001/chrome/browser/content...
File
chrome/browser/content_settings/content_settings_internal_extension_provider.cc
(right):

https://codereview.chromium.org/2839373003/diff/100001/chrome/browser/content...
chrome/browser/content_settings/content_settings_internal_extension_provider.cc:27:
using extensions::UnloadedExtensionReason;
On 2017/05/01 14:08:57, Devlin wrote:
> I know this was just copy-replace, but it looks like this isn't needed.

Done.

https://codereview.chromium.org/2839373003/diff/100001/extensions/common/exte...
File extensions/common/extension.h (right):

https://codereview.chromium.org/2839373003/diff/100001/extensions/common/exte...
extensions/common/extension.h:518: enum UnloadedExtensionReason {
On 2017/05/01 14:08:57, Devlin wrote:
> We should make this an enum class; otherwise the values are defined at the
scope
> of the extensions namespace (e.g., extensions::REASON_UNDEFINED would work -
> which is a little too vague).
> 
> Two options:
> 1) Just make this an enum class in this file; everything should still work
with
> no other changes (because you've specified all the enums with the more
targeted
> scoping already in the code).
> 2) Make this an enum class and remove the REASON_ prefix from the values. 
This
> would be a *bit* cleaner, because UnloadedExtensionReason::REASON_FOO is a
> little redundant compared to UnloadedExtensionReason::FOO.  If you have a
> find-replace script that works well, this shouldn't be too hard
> (s/UnloadedExtensionReason::REASON_/UnloadedExtensionReason::), but if there's
> any trouble, then don't worry about it - it's a very small detail. :)

Yeah, we should. (sorry I didn't catch that)
I'll take option#2 to remove duplicated word, 'reason'.

https://codereview.chromium.org/2839373003/diff/120001/extensions/common/exte...
File extensions/common/extension.h (right):

https://codereview.chromium.org/2839373003/diff/120001/extensions/common/exte...
extensions/common/extension.h:524: REASON_UNDEFINED,         // Undefined state
used to initialize variables.
devlin@, can I remove this in this CL? I think we always should specify the
exact reason when unloading extension.

[Devlin, 2017-05-02 20:49:27]

lgtm, this looks great!  Thanks for the cleanup!!

https://codereview.chromium.org/2839373003/diff/120001/extensions/common/exte...
File extensions/common/extension.h (right):

https://codereview.chromium.org/2839373003/diff/120001/extensions/common/exte...
extensions/common/extension.h:524: REASON_UNDEFINED,         // Undefined state
used to initialize variables.
On 2017/05/02 07:52:52, limasdf wrote:
> devlin@, can I remove this in this CL? I think we always should specify the
> exact reason when unloading extension.

Sounds good to me!

[sky, 2017-05-03 03:25:23]

LGTM

[limasdf, 2017-05-03 04:04:55]

Thanks for the review.

https://codereview.chromium.org/2839373003/diff/120001/extensions/common/exte...
File extensions/common/extension.h (right):

https://codereview.chromium.org/2839373003/diff/120001/extensions/common/exte...
extensions/common/extension.h:524: REASON_UNDEFINED,         // Undefined state
used to initialize variables.
On 2017/05/02 20:49:27, Devlin wrote:
> On 2017/05/02 07:52:52, limasdf wrote:
> > devlin@, can I remove this in this CL? I think we always should specify the
> > exact reason when unloading extension.
> 
> Sounds good to me!

I'll not remove it since it is being used from
extension_service_test_with_install.cc

[limasdf, 2017-05-03 04:05:09]

The CQ bit was checked by limasdf@gmail.com

[commit-bot: I haz the power, 2017-05-03 04:05:26]

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-05-03 04:19:01]

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

[commit-bot: I haz the power, 2017-05-03 04:19:03]

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...)

[limasdf, 2017-05-03 04:23:16]

limasdf@gmail.com changed reviewers:
+ dbeam@chromium.org

[limasdf, 2017-05-03 08:50:31]

Hi Dan,

Would you take a look chrome/browser/ui/webui/options/* ? Changes are
mechanical.

[Dan Beam, 2017-05-03 18:16:36]

lgtm

[limasdf, 2017-05-03 18:17:10]

thanks!

[limasdf, 2017-05-03 18:17:22]

The CQ bit was checked by limasdf@gmail.com

[limasdf, 2017-05-03 18:17:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
rdevlin.cronin@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2839373003/#ps240001
(title: "Rebase")

[commit-bot: I haz the power, 2017-05-03 18:18:05]

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-05-03 19:17:21]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1493835442525510,
"parent_rev": "46d21233e6fd76a8b76952d3ac952f615e2d9c1a", "commit_rev":
"0deef20444dd64caa41f81b5175925d9a5bf3649"}

[commit-bot: I haz the power, 2017-05-03 19:18:31]

Description was changed from

==========
struct UnloadedExtensionInfo -> enum UnloadedExtensionInfoReason

We don't need to keep struct UnloadedExtensionInfo.
So removing it and use enum UnloadedExtensionInfoReason instead.

TEST=Everything runs smoothly
BUG=715144
==========

to

==========
struct UnloadedExtensionInfo -> enum UnloadedExtensionInfoReason

We don't need to keep struct UnloadedExtensionInfo.
So removing it and use enum UnloadedExtensionInfoReason instead.

TEST=Everything runs smoothly
BUG=715144

Review-Url: https://codereview.chromium.org/2839373003
Cr-Commit-Position: refs/heads/master@{#469065}
Committed:
https://chromium.googlesource.com/chromium/src/+/0deef20444dd64caa41f81b51759...
==========

[commit-bot: I haz the power, 2017-05-03 19:18:33]

Committed patchset #10 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/0deef20444dd64caa41f81b51759...