Issue 12095052: Move app_launcher.* out of chrome/browser/extensions and into apps/

Issue 12095052: Move app_launcher.* out of chrome/browser/extensions and into apps/ (Closed)

Created:
7 years, 10 months ago by benwells

Modified:
7 years, 10 months ago

Reviewers:
Yoyo Zhou, koz (OOO until 15th September), Ben Goodger (Google)

CC:
chromium-reviews, dbeam+watch-ntp_chromium.org, Aaron Boodman, pedrosimonetti+watch_chromium.org, estade+watch_chromium.org, chromium-apps-reviews_chromium.org, chrome-apps-syd-reviews_chromium.org, tfarina, Jói

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Move app_launcher.* out of chrome/browser/extensions and into apps/ This change also moves some UI code from chrome/browser/extensions into /chrome/browser/ui/, and cleans up the app_launcher.* code. BUG=159366 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181875

Patch Set 1 #

Patch Set 2 : Win compile #

Patch Set 3 : Win, android compile #

Patch Set 4 : Rebase #

Patch Set 5 : Moved switches back #

Total comments: 16

Patch Set 6 : Rebase #

Patch Set 7 : Feedback #

Patch Set 8 : Fix bad rebase #

Patch Set 9 : More build fixes #

Patch Set 10 : Gah! #

Patch Set 11 : ReBasey #

Patch Set 12 : Beserk Q. More rebase #

Created: 7 years, 10 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+166 lines, -647 lines)			Patch
M	apps/DEPS	View	1 2 3 4 5 6 7	1 chunk	+5 lines, -1 line	0 comments	Download
A +	apps/app_launcher.h	View	1 2 3 4 5 6 7	1 chunk	+16 lines, -12 lines	0 comments	Download
A +	apps/app_launcher.cc	View	1 2 3 4 5 6 7 8 9 10 11	4 chunks	+32 lines, -46 lines	0 comments	Download
M	apps/apps.gypi	View	1 2 3 4 5 6 7	1 chunk	+6 lines, -0 lines	0 comments	Download
A	apps/pref_names.h	View	1 2 3 4 5 6	1 chunk	+18 lines, -0 lines	0 comments	Download
A	apps/pref_names.cc	View	1 2 3 4 5 6	1 chunk	+17 lines, -0 lines	0 comments	Download
A	apps/prefs.h	View	1 2 3 4 5 6	1 chunk	+17 lines, -0 lines	0 comments	Download
A	apps/prefs.cc	View	1 2 3 4 5 6 7 8 9 10 11	1 chunk	+25 lines, -0 lines	0 comments	Download
A +	chrome/browser/extensions/api/DEPS	View	1 2 3 4 5	1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/extensions/api/webstore_private/webstore_private_api.cc	View	1 2 3 4 5 6 7 8 9 10 11	4 chunks	+3 lines, -3 lines	0 comments	Download
D	chrome/browser/extensions/app_launcher.h	View	1 2 3 4 5	1 chunk	+0 lines, -34 lines	0 comments	Download
D	chrome/browser/extensions/app_launcher.cc	View	1 2 3 4 5 6 7 8 9 10 11	1 chunk	+0 lines, -121 lines	0 comments	Download
D	chrome/browser/extensions/extension_install_ui_android.h	View		1 chunk	+0 lines, -26 lines	0 comments	Download
D	chrome/browser/extensions/extension_install_ui_android.cc	View		1 chunk	+0 lines, -55 lines	0 comments	Download
D	chrome/browser/extensions/extension_install_ui_default.h	View		1 chunk	+0 lines, -45 lines	0 comments	Download
D	chrome/browser/extensions/extension_install_ui_default.cc	View	1 2 3 4 5	1 chunk	+0 lines, -271 lines	0 comments	Download
M	chrome/browser/prefs/browser_prefs.cc	View	1 2 3 4 5 6 7 8 9 10 11	3 chunks	+2 lines, -2 lines	0 comments	Download
A +	chrome/browser/ui/android/extensions/extension_install_ui_android.h	View		2 chunks	+3 lines, -3 lines	0 comments	Download
A +	chrome/browser/ui/android/extensions/extension_install_ui_android.cc	View	1 2	1 chunk	+1 line, -1 line	0 comments	Download
A +	chrome/browser/ui/extensions/extension_install_ui_default.h	View		2 chunks	+3 lines, -3 lines	0 comments	Download
A +	chrome/browser/ui/extensions/extension_install_ui_default.cc	View	1 2 3 4 5	2 chunks	+3 lines, -3 lines	0 comments	Download
M	chrome/browser/ui/webui/ntp/new_tab_page_handler.cc	View	1 2 3 4 5 6 7 8 9 10 11	2 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/webui/ntp/new_tab_ui.cc	View	1 2 3 4 5 6 7 8 9 10 11	3 chunks	+4 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/webui/ntp/ntp_resource_cache.cc	View	1 2 3 4 5 6 7 8 9 10 11	3 chunks	+2 lines, -2 lines	0 comments	Download
M	chrome/chrome_browser_extensions.gypi	View	1 2 3 4 5 6 7 8 9 10 11	4 chunks	+0 lines, -8 lines	0 comments	Download
M	chrome/chrome_browser_ui.gypi	View	1 2 3 4 5 6 7 8 9 10 11	3 chunks	+6 lines, -0 lines	0 comments	Download
M	chrome/common/pref_names.h	View	1 2 3 4 5 6 7 8 9 10 11	1 chunk	+0 lines, -2 lines	0 comments	Download
M	chrome/common/pref_names.cc	View	1 2 3 4 5 6 7 8 9 10 11	1 chunk	+0 lines, -4 lines	0 comments	Download

Messages

Total messages: 27 (0 generated)

Expand Messages | Collapse Messages

benwells

koz: stuff in apps/ in particular app_launcher.* which I've cleaned up. yoz: moving the extensions_install_ui_* ...

7 years, 10 months ago (2013-01-30 06:04:17 UTC) #1

koz (OOO until 15th September)

On 2013/01/30 06:04:17, benwells wrote: > koz: stuff in apps/ in particular app_launcher.* which I've ...

7 years, 10 months ago (2013-01-30 06:23:51 UTC) #2

benwells

+ben for owners review for non-apps, non-extensions; plus anything else you wanna look at (e.g. ...

7 years, 10 months ago (2013-01-31 02:11:30 UTC) #4

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/12095052/6001

7 years, 10 months ago (2013-01-31 22:16:23 UTC) #6

commit-bot: I haz the power

Failed to request the patch to try. Please note that binary files are still unsupported ...

7 years, 10 months ago (2013-01-31 22:16:50 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/12095052/6001

7 years, 10 months ago (2013-02-01 03:38:37 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, 10 months ago (2013-02-01 04:39:44 UTC) #9

tfarina

Not sure why I wasn't copied, may it was before I landed my apps watchlist ...

7 years, 10 months ago (2013-02-01 15:08:24 UTC) #10

tfarina

https://codereview.chromium.org/12095052/diff/18001/apps/prefs.cc File apps/prefs.cc (right): https://codereview.chromium.org/12095052/diff/18001/apps/prefs.cc#newcode8 apps/prefs.cc:8: #include "chrome/browser/prefs/pref_service.h" so is this allowed for now? I ...

7 years, 10 months ago (2013-02-01 15:11:24 UTC) #11

tfarina

https://codereview.chromium.org/12095052/diff/18001/apps/DEPS File apps/DEPS (right): https://codereview.chromium.org/12095052/diff/18001/apps/DEPS#newcode3 apps/DEPS:3: "+content", content/ should not be allowed to depend on ...

7 years, 10 months ago (2013-02-01 15:32:37 UTC) #12

Jói

PrefService except for PrefServiceSyncable will move to base/prefs very soon, so should be OK as ...

7 years, 10 months ago (2013-02-01 16:06:01 UTC) #13

benwells

7 years, 10 months ago (2013-02-08 07:18:00 UTC) #14

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/12095052/20026

7 years, 10 months ago (2013-02-08 07:19:37 UTC) #15

commit-bot: I haz the power

Sorry for I got bad news for ya. Compile failed with a clobber build on ...

7 years, 10 months ago (2013-02-08 07:48:02 UTC) #16

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/12095052/30003

7 years, 10 months ago (2013-02-08 07:55:41 UTC) #17

commit-bot: I haz the power

Sorry for I got bad news for ya. Compile failed with a clobber build on ...

7 years, 10 months ago (2013-02-08 08:23:10 UTC) #18

Jói

FYI, the RegisterXYZ stuff from Prefs will be moving to base/prefs/ in the next week ...

7 years, 10 months ago (2013-02-08 10:26:42 UTC) #19

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/12095052/30036

7 years, 10 months ago (2013-02-08 22:53:51 UTC) #20

commit-bot: I haz the power

Sorry for I got bad news for ya. Compile failed with a clobber build on ...

7 years, 10 months ago (2013-02-09 00:13:17 UTC) #21

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/12095052/29035

7 years, 10 months ago (2013-02-11 10:35:30 UTC) #22

commit-bot: I haz the power

The commit queue went berserk retrying too often for a seemingly flaky test. Builder is ...

7 years, 10 months ago (2013-02-11 17:59:53 UTC) #23

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/12095052/29035

7 years, 10 months ago (2013-02-11 20:39:36 UTC) #24

commit-bot: I haz the power

Failed to apply patch for apps/app_launcher.cc: While running patch -p1 --forward --force --no-backup-if-mismatch; A apps/app_launcher.cc ...

7 years, 10 months ago (2013-02-11 20:39:46 UTC) #25

Failed to apply patch for apps/app_launcher.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         apps/app_launcher.cc
  Copied chrome/browser/extensions/app_launcher.cc -> apps/app_launcher.cc
  patching file apps/app_launcher.cc
  Hunk #1 FAILED at 1.
  1 out of 4 hunks FAILED -- saving rejects to file apps/app_launcher.cc.rej

Patch:   NR  chrome/browser/extensions/app_launcher.cc->apps/app_launcher.cc
Index: apps/app_launcher.cc
diff --git a/chrome/browser/extensions/app_launcher.cc b/apps/app_launcher.cc
similarity index 71%
rename from chrome/browser/extensions/app_launcher.cc
rename to apps/app_launcher.cc
index
385517725b5be28c0b4f5be2659400839b5cd963..dd4e1a7254440cd3ff37e1f6a9d3545c44dceb7e
100644
--- a/chrome/browser/extensions/app_launcher.cc
+++ b/apps/app_launcher.cc
@@ -1,16 +1,15 @@
-// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/extensions/app_launcher.h"
+#include "apps/app_launcher.h"
 
+#include "apps/pref_names.h"
 #include "base/command_line.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/prefs/pref_registry_simple.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/chrome_switches.h"
-#include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 
 #if defined(OS_WIN)
@@ -18,30 +17,10 @@
 #include "chrome/installer/util/browser_distribution.h"
 #endif
 
-namespace extensions {
+namespace apps {
 
 namespace {
 
-#if defined(OS_WIN)
-void UpdatePrefAndCallCallbackOnUI(
-    bool result,
-    const OnAppLauncherEnabledCompleted& completion_callback) {
-  PrefService* prefs = g_browser_process->local_state();
-  prefs->SetBoolean(prefs::kAppLauncherIsEnabled, result);
-  completion_callback.Run(result);
-}
-
-void IsAppLauncherInstalledOnBlockingPool(
-    const OnAppLauncherEnabledCompleted& completion_callback) {
- 
DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
-  bool result = chrome_launcher_support::IsAppLauncherPresent();
-  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
-      base::Bind(UpdatePrefAndCallCallbackOnUI, result, completion_callback));
-}
-#endif
-
-}  // namespace
-
 enum AppLauncherState {
   APP_LAUNCHER_UNKNOWN,
   APP_LAUNCHER_ENABLED,
@@ -66,7 +45,31 @@ AppLauncherState SynchronousAppLauncherChecks() {
 #endif
 }
 
-void UpdateIsAppLauncherEnabled(
+#if defined(OS_WIN)
+void UpdatePrefAndCallCallbackOnUI(
+    bool result,
+    const OnAppLauncherEnabledCompleted& completion_callback) {
+  PrefService* prefs = g_browser_process->local_state();
+  prefs->SetBoolean(prefs::kAppLauncherIsEnabled, result);
+  completion_callback.Run(result);
+}
+
+void IsAppLauncherInstalledOnBlockingPool(
+    const OnAppLauncherEnabledCompleted& completion_callback) {
+ 
DCHECK(content::BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread());
+  bool result = chrome_launcher_support::IsAppLauncherPresent();
+  content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+      base::Bind(UpdatePrefAndCallCallbackOnUI, result, completion_callback));
+}
+#endif
+
+}  // namespace
+
+bool MaybeIsAppLauncherEnabled() {
+  return SynchronousAppLauncherChecks() == APP_LAUNCHER_ENABLED;
+}
+
+void GetIsAppLauncherEnabled(
     const OnAppLauncherEnabledCompleted& completion_callback) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
@@ -92,30 +95,12 @@ void UpdateIsAppLauncherEnabled(
 #endif
 }
 
-bool IsAppLauncherEnabled() {
+bool WasAppLauncherEnabled() {
   PrefService* prefs = g_browser_process->local_state();
-  // In some tests, the prefs aren't initialised, but the NTP still needs to
-  // work.
+  // In some tests, the prefs aren't initialised.
   if (!prefs)
     return SynchronousAppLauncherChecks() == APP_LAUNCHER_ENABLED;
   return prefs->GetBoolean(prefs::kAppLauncherIsEnabled);
 }
 
-namespace app_launcher {
-
-void RegisterPrefs(PrefRegistrySimple* registry) {
-  // If it is impossible to synchronously determine whether the app launcher is
-  // enabled, assume it is disabled. Anything that needs to know the absolute
-  // truth should call UpdateIsAppLauncherEnabled().
-  //
-  // This pref is just a cache of the value from the registry from last time
-  // Chrome ran. To avoid having the NTP block on a registry check, it guesses
-  // that the value hasn't changed since last time it was checked, using this
-  // preference.
-  bool is_enabled = SynchronousAppLauncherChecks() == APP_LAUNCHER_ENABLED;
-  registry->RegisterBooleanPref(prefs::kAppLauncherIsEnabled, is_enabled);
-}
-
-}  // namespace app_launcher
-
-}  // namespace extensions
+}  // namespace apps

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/benwells@chromium.org/12095052/30052

7 years, 10 months ago (2013-02-12 02:44:03 UTC) #26

Message was sent while issue was closed.

Change committed as 181875

Expand Messages | Collapse Messages