Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(971)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 938603002: Disable bookmark apps on non-ChromeOS platforms. (Closed)

Created:
5 years, 10 months ago by benwells
Modified:
5 years, 9 months ago
Reviewers:
Lei Zhang, jackhou1, tapted
CC:
chromium-reviews, chrome-apps-syd-reviews_chromium.org, chromium-apps-reviews_chromium.org, tfarina, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Disable bookmark apps on non-ChromeOS platforms. The world isn't quite ready for them.... BUG=459504

Patch Set 1 #

Total comments: 4

Patch Set 2 : Feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -10 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M chrome/browser/chrome_content_browser_client.cc View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/extension_util.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/ui/app_list/app_list_service_impl_browsertest.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/chrome_switches.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/common/chrome_switches.cc View 1 2 chunks +4 lines, -1 line 0 comments Download
M chrome/renderer/web_apps.cc View 1 chunk +8 lines, -2 lines 0 comments Download
M chrome/test/data/extensions/api_test/management/test/launchType.js View 1 chunk +5 lines, -0 lines 0 comments Download

Dependent Patchsets:

Issue 1203023002 Patch 1
Issue 1203023002 Patch 20001
Issue 1206163002 Patch 1

Messages

Total messages: 11 (5 generated)
benwells
5 years, 10 months ago (2015-02-18 07:21:27 UTC) #2
Lei Zhang
lgtm Feel free to land this now and fix the nits later. https://codereview.chromium.org/938603002/diff/1/chrome/common/chrome_switches.cc File chrome/common/chrome_switches.cc ...
5 years, 10 months ago (2015-02-18 20:12:29 UTC) #3
benwells
https://codereview.chromium.org/938603002/diff/1/chrome/common/chrome_switches.cc File chrome/common/chrome_switches.cc (right): https://codereview.chromium.org/938603002/diff/1/chrome/common/chrome_switches.cc#newcode468 chrome/common/chrome_switches.cc:468: const char kEnableNewBookmarkApps[] = "enable-new-bookmark-apps"; On 2015/02/18 20:12:29, Lei ...
5 years, 10 months ago (2015-02-18 20:26:54 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/938603002/20001
5 years, 10 months ago (2015-02-18 20:28:03 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/35578)
5 years, 10 months ago (2015-02-18 23:41:11 UTC) #9
tapted
5 years, 10 months ago (2015-02-19 03:54:28 UTC) #11 
On 2015/02/18 23:41:11, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

Since Ben is in a tent with no electricity and this needs to land before
branch... I've checked with Ben over IM and taken this over. The COLLABORATOR=
feature seems to be broken, so I've put up a new CL TBR in
https://codereview.chromium.org/937843004 . I had a close look at these
failures. None of them are fatal, just different expectations because of the
flag change.

E.g.

ExtensionManagementApiTest.LaunchTabApp
 -> fails because it wants the different test expectations #ifdef MAC, deleted
in https://codereview.chromium.org/777543002

ExtensionManagementApiTest.LaunchType
 -> fails because of the fallback mode in
https://codereview.chromium.org/921103003  [this test actually should have
failed on Mac since it was first added]

AppShimMenuControllerBrowserTest.HostedAppHasAdditionalEditMenuItems
AppShimInteractiveTest.HostedAppLaunch
 -> these fail because they are specifically for this feature, which is getting
disabled.

I'll get jack to take a look at the changes, since he's most familiar with
hosted apps on Mac. Otherwise the CL is the same as what's approved here.

Powered by Google App Engine
This is Rietveld 408576698