Rename PACKAGED_APP references to LEGACY_PACKAGED_APP

Rename PACKAGED_APP references to LEGACY_PACKAGED_APP

BUG=151656
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=159211

[Mihai Parparita -not on Chrome, 2012-09-14 18:19:53]

Nit: please file a bug about this and add it to the BUG= line, or remove
the empty BUG= line

Mihai

On Fri, Sep 14, 2012 at 11:11 AM, <grv@chromium.org> wrote:

> Reviewers: Antony Sargent,
>
> Description:
> Rename PACKAGED_APP references to LEGACY_PACKAGED_APP
>
>
> BUG=
>
>
> Please review this at
https://chromiumcodereview.**appspot.com/10909239/<https://chromiumcodereview...
>
> SVN Base:
http://git.chromium.org/**chromium/src.git@master<http://git.chromium.org/chr...
>
> Affected files:
>   M chrome/browser/chrome_content_**browser_client.cc
>   M chrome/browser/extensions/api/**management/management_api.cc
>   M chrome/browser/extensions/**convert_web_app_unittest.cc
>   M chrome/browser/extensions/**installed_loader.cc
>   M chrome/browser/extensions/**settings/settings_frontend_**unittest.cc
>   M chrome/browser/extensions/**settings/settings_sync_**unittest.cc
>   M chrome/browser/extensions/**settings/settings_test_util.cc
>   M chrome/browser/extensions/**updater/extension_downloader.**cc
>   M chrome/browser/renderer_host/**chrome_render_view_host_**observer.cc
>   M chrome/browser/sync/test/**integration/sync_extension_**helper.cc
>   M chrome/common/extensions/csp_**validator.cc
>   M chrome/common/extensions/csp_**validator_unittest.cc
>   M chrome/common/extensions/**extension.h
>   M chrome/common/extensions/**extension.cc
>   M chrome/common/extensions/**features/feature.cc
>   M chrome/common/extensions/**features/feature_unittest.cc
>   M chrome/common/extensions/**features/simple_feature_**
> provider_unittest.cc
>   M chrome/common/extensions/**manifest.h
>   M chrome/common/extensions/**manifest.cc
>   M chrome/common/extensions/**manifest_unittest.cc
>
>
>

[asargent_no_longer_on_chrome, 2012-09-19 17:50:35]

Looks fine overall - just add the BUG= line to the description as Mihai
mentioned (use the "Edit Issue" command from the codereview web page to do
this).

[Mihai Parparita -not on Chrome, 2012-09-26 22:05:15]

LGTM (if this CL was still waiting for me)

[commit-bot: I haz the power, 2012-09-26 22:06:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grv@chromium.org/10909239/1

[commit-bot: I haz the power, 2012-09-26 22:06:54]

Presubmit check for 10909239-1 failed and returned exit status 1.


All integration tests passed!
Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome
    chrome/browser/sync

Presubmit checks took 2.1s to calculate.

[Gaurav, 2012-09-26 22:21:21]

@akalin: files browser/sync/test/integration/

@thakis: browser/chrome_content_browser_client.cc,     
     browser/renderer_host/chrome_render_view_host_observer.cc

Thanks

[Nico, 2012-09-27 02:15:14]

http://codereview.chromium.org/10909239/diff/1/chrome/common/extensions/exten...
File chrome/common/extensions/extension.h (right):

http://codereview.chromium.org/10909239/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension.h:134: TYPE_LEGACY_PACKAGED_APP,
What's the motivation behind this change? Do you want to introduce a new
TYPE_PACKAGED_APP enum for a new packaged app concept? In that case _V1 and _V2
might be better names (v2 might become legacy too eventually after all).

If it's to distinguish this from TYPE_PLATFORM_APP, maybe you can add a comment
to that end. ("// This is marked legacy because platform apps are preferred. For
backwards compatibility, we can't remove support for packaged apps") or
something like that.

[Mihai Parparita -not on Chrome, 2012-09-27 17:40:50]

On Wed, Sep 26, 2012 at 7:15 PM, <thakis@chromium.org> wrote:

> http://codereview.chromium.**org/10909239/diff/1/chrome/**
>
common/extensions/extension.h<http://codereview.chromium.org/10909239/diff/1/chrome/common/extensions/extension.h>
> File chrome/common/extensions/**extension.h (right):
>
> http://codereview.chromium.**org/10909239/diff/1/chrome/**
>
common/extensions/extension.h#**newcode134<http://codereview.chromium.org/10909239/diff/1/chrome/common/extensions/extension.h#newcode134>
> chrome/common/extensions/**extension.h:134: TYPE_LEGACY_PACKAGED_APP,
> What's the motivation behind this change? Do you want to introduce a new
> TYPE_PACKAGED_APP enum for a new packaged app concept? In that case _V1
> and _V2 might be better names (v2 might become legacy too eventually
> after all).
>

Platform/v2 apps are currently referred to as "packaged apps" publicly
(with the old packaged apps being deprecated and referred to as "legacy"),
so the rationale for this change was that someone new to the codebase would
come across code called "packaged apps" and rightfully assume that it's
about what the public docs were about.

Once we're 100% set on the public name (Sundar has asked if we can think of
a catchier name than "packaged apps") I would be up for a grand renaming CL
where we replace references to platform apps with whatever the public name
is.

Mihai

[Mihai Parparita -not on Chrome, 2012-09-27 17:41:17]

Gaurav: when uploading a new patch, please indicate what's new about it and
where the reviewer should look (in this case, it looks like you added
Nico's suggested comment to extension.h).

Mihai

On Thu, Sep 27, 2012 at 10:23 AM, <grv@chromium.org> wrote:

>
http://codereview.chromium.**org/10909239/<http://codereview.chromium.org/109...
>

[Nico, 2012-09-28 01:38:14]

lgtm

+1 on adding  a "Done" note when uploading a new patch set.

http://codereview.chromium.org/10909239/diff/4002/chrome/common/extensions/ex...
File chrome/common/extensions/extension.h (right):

http://codereview.chromium.org/10909239/diff/4002/chrome/common/extensions/ex...
chrome/common/extensions/extension.h:135: // backwards compatibility, we can't
remove support for packaged apps
nit: trailing .

[commit-bot: I haz the power, 2012-09-28 01:38:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grv@chromium.org/10909239/4002

[commit-bot: I haz the power, 2012-09-28 04:40:12]

Change committed as 159211