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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 25034003: Moved apps metro code from apps to chrome/browser/apps. (Closed)

Created:
7 years, 2 months ago by benwells
Modified:
7 years, 2 months ago
Reviewers:
tapted, sky
CC:
chromium-reviews, tfarina, extensions-reviews_chromium.org, chromium-apps-reviews_chromium.org, chrome-apps-syd-reviews_chromium.org
Visibility:
Public.

Description

Moved apps metro code from apps to chrome/browser/apps. This code is chrome specific and does not belong in the generic apps component. BUG=159366 TEST=Run apps and app launcher on Windows 8, in these cases: * when chrome is not running, * when chrome is already running in metro mode, * when chrome is already running and is not in metro mode. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=226705

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : Removed unused DEP #

Total comments: 4

Patch Set 5 : Fix stuffup #

Total comments: 2

Patch Set 6 : Feedback #

Patch Set 7 : Rebase and fix unit test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+83 lines, -171 lines) Patch
M apps/DEPS View 1 2 3 1 chunk +0 lines, -3 lines 0 comments Download
D apps/app_launch_for_metro_restart_win.h View 1 chunk +0 lines, -26 lines 0 comments Download
D apps/app_launch_for_metro_restart_win.cc View 1 chunk +0 lines, -93 lines 0 comments Download
M apps/app_shim/DEPS View 1 1 chunk +1 line, -0 lines 0 comments Download
M apps/apps.gypi View 1 chunk +0 lines, -2 lines 0 comments Download
M apps/apps_client.h View 1 2 chunks +9 lines, -0 lines 0 comments Download
M apps/launcher.cc View 1 3 chunks +2 lines, -10 lines 0 comments Download
M apps/pref_names.h View 1 chunk +0 lines, -2 lines 0 comments Download
M apps/pref_names.cc View 1 chunk +0 lines, -9 lines 0 comments Download
M apps/prefs.h View 1 chunk +0 lines, -3 lines 0 comments Download
M apps/prefs.cc View 1 chunk +0 lines, -7 lines 0 comments Download
A + chrome/browser/apps/app_launch_for_metro_restart_win.h View 1 2 chunks +9 lines, -5 lines 0 comments Download
A + chrome/browser/apps/app_launch_for_metro_restart_win.cc View 1 2 3 chunks +10 lines, -3 lines 0 comments Download
M chrome/browser/apps/chrome_apps_client.h View 1 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/apps/chrome_apps_client.cc View 1 2 3 4 2 chunks +22 lines, -0 lines 0 comments Download
M chrome/browser/prefs/browser_prefs.cc View 1 2 3 4 5 3 chunks +8 lines, -1 line 0 comments Download
M chrome/browser/ui/app_list/app_list_service_unittest.cc View 1 2 3 4 5 6 2 chunks +0 lines, -3 lines 0 comments Download
M chrome/browser/ui/apps/app_metro_infobar_delegate_win.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/ui/startup/startup_browser_creator_impl.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/chrome_browser_extensions.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/pref_names.h View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/pref_names.cc View 1 2 3 4 5 6 1 chunk +9 lines, -0 lines 0 comments Download

Messages

Total messages: 14 (0 generated)
benwells
7 years, 2 months ago (2013-09-30 09:48:38 UTC) #1
tapted
https://codereview.chromium.org/25034003/diff/14001/chrome/browser/apps/chrome_apps_client.cc File chrome/browser/apps/chrome_apps_client.cc (right): https://codereview.chromium.org/25034003/diff/14001/chrome/browser/apps/chrome_apps_client.cc#newcode45 chrome/browser/apps/chrome_apps_client.cc:45: return true; Should this be false? https://codereview.chromium.org/25034003/diff/14001/chrome/browser/apps/chrome_apps_client.cc#newcode48 chrome/browser/apps/chrome_apps_client.cc:48: return ...
7 years, 2 months ago (2013-10-01 03:24:49 UTC) #2
benwells
https://codereview.chromium.org/25034003/diff/14001/chrome/browser/apps/chrome_apps_client.cc File chrome/browser/apps/chrome_apps_client.cc (right): https://codereview.chromium.org/25034003/diff/14001/chrome/browser/apps/chrome_apps_client.cc#newcode45 chrome/browser/apps/chrome_apps_client.cc:45: return true; On 2013/10/01 03:24:49, tapted wrote: > Should ...
7 years, 2 months ago (2013-10-01 09:26:36 UTC) #3
tapted
Cool - lgtm
7 years, 2 months ago (2013-10-01 09:57:33 UTC) #4
benwells
+sky for chrome/browser/prefs and chrome/browser/ui/startup
7 years, 2 months ago (2013-10-01 10:00:59 UTC) #5
sky
LGTM https://codereview.chromium.org/25034003/diff/18001/chrome/browser/prefs/browser_prefs.cc File chrome/browser/prefs/browser_prefs.cc (right): https://codereview.chromium.org/25034003/diff/18001/chrome/browser/prefs/browser_prefs.cc#newcode221 chrome/browser/prefs/browser_prefs.cc:221: #if defined(OS_WIN) move with rest of platform specific ...
7 years, 2 months ago (2013-10-01 17:38:54 UTC) #6
benwells
https://codereview.chromium.org/25034003/diff/18001/chrome/browser/prefs/browser_prefs.cc File chrome/browser/prefs/browser_prefs.cc (right): https://codereview.chromium.org/25034003/diff/18001/chrome/browser/prefs/browser_prefs.cc#newcode221 chrome/browser/prefs/browser_prefs.cc:221: #if defined(OS_WIN) On 2013/10/01 17:38:54, sky wrote: > move ...
7 years, 2 months ago (2013-10-02 00:57:59 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/25034003/30001
7 years, 2 months ago (2013-10-02 00:58:12 UTC) #8
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 2 months ago (2013-10-02 01:38:21 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/25034003/30001
7 years, 2 months ago (2013-10-02 09:50:50 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/25034003/58001
7 years, 2 months ago (2013-10-02 10:28:36 UTC) #11
commit-bot: I haz the power
Retried try job too often on win_rel for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&number=204527
7 years, 2 months ago (2013-10-02 13:01:58 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/25034003/58001
7 years, 2 months ago (2013-10-03 04:20:26 UTC) #13
commit-bot: I haz the power
7 years, 2 months ago (2013-10-03 08:14:07 UTC) #14
Message was sent while issue was closed. 
Change committed as 226705

Powered by Google App Engine
This is Rietveld 408576698