Move apps_shortcut_manager into src/apps

Move apps_shortcut_manager into src/apps

BUG=159366
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=186648

[benwells, 2013-03-01 06:53:43]

Notes:
1. the next step for this area of apps is to extract out the basic shortcut bits
of shell_integration and web_app code into a shortcut_util file in apps. Stuff
specific to shortcuts to web sites would stay in chrome
2. it seems image.h was included via transitive includes through extension
service and app_shortcut_manager which necessitated adding in the extra
includes.

[benwells, 2013-03-03 23:51:01]

On 2013/03/01 06:53:43, benwells wrote:
> Notes:
> 1. the next step for this area of apps is to extract out the basic shortcut
bits
> of shell_integration and web_app code into a shortcut_util file in apps. Stuff
> specific to shortcuts to web sites would stay in chrome
> 2. it seems image.h was included via transitive includes through extension
> service and app_shortcut_manager which necessitated adding in the extra
> includes.

+the real Ben Goodger for review.

[tfarina, 2013-03-03 23:57:49]

https://codereview.chromium.org/12378037/diff/8016/apps/apps.gypi
File apps/apps.gypi (right):

https://codereview.chromium.org/12378037/diff/8016/apps/apps.gypi#newcode48
apps/apps.gypi:48: '../build/linux/system.gyp:gdk',
why do you need this now?

https://codereview.chromium.org/12378037/diff/8016/apps/shortcut_manager_fact...
File apps/shortcut_manager_factory.h (right):

https://codereview.chromium.org/12378037/diff/8016/apps/shortcut_manager_fact...
apps/shortcut_manager_factory.h:8: #include "base/memory/singleton.h"
forward declare this:

template <typename T> struct DefaultSingletonTraits; 

and include in the source file instead.

https://codereview.chromium.org/12378037/diff/8016/apps/shortcut_manager_fact...
apps/shortcut_manager_factory.h:11: class Profile;
no need to forward declare this, it comes from profile_keyed_service_factory.h

[Matt Giuca, 2013-03-04 00:10:46]

https://codereview.chromium.org/12378037/diff/8016/chrome/browser/extensions/...
File chrome/browser/extensions/api/extension_action/extension_action_api.cc
(right):

https://codereview.chromium.org/12378037/diff/8016/chrome/browser/extensions/...
chrome/browser/extensions/api/extension_action/extension_action_api.cc:34:
#include "ui/gfx/image/image.h"
Why do you add this #include to a bunch of files? I assume it's an include what
you use thing, but it doesn't seem related to this CL.

[benwells, 2013-03-04 01:30:03]

https://codereview.chromium.org/12378037/diff/8016/apps/apps.gypi
File apps/apps.gypi (right):

https://codereview.chromium.org/12378037/diff/8016/apps/apps.gypi#newcode48
apps/apps.gypi:48: '../build/linux/system.gyp:gdk',
On 2013/03/03 23:57:49, tfarina wrote:
> why do you need this now?

good question. it was needed because i am now including profile.h, which
includes stuff something which needs this.

I don't think shortcut_manager needs profile.h, so will remove that instead of
adding this dependency.

https://codereview.chromium.org/12378037/diff/8016/apps/shortcut_manager_fact...
File apps/shortcut_manager_factory.h (right):

https://codereview.chromium.org/12378037/diff/8016/apps/shortcut_manager_fact...
apps/shortcut_manager_factory.h:8: #include "base/memory/singleton.h"
On 2013/03/03 23:57:49, tfarina wrote:
> forward declare this:
> 
> template <typename T> struct DefaultSingletonTraits; 
> 
> and include in the source file instead.

Done.

https://codereview.chromium.org/12378037/diff/8016/apps/shortcut_manager_fact...
apps/shortcut_manager_factory.h:11: class Profile;
On 2013/03/03 23:57:49, tfarina wrote:
> no need to forward declare this, it comes from profile_keyed_service_factory.h

Done.

https://codereview.chromium.org/12378037/diff/8016/chrome/browser/extensions/...
File chrome/browser/extensions/api/extension_action/extension_action_api.cc
(right):

https://codereview.chromium.org/12378037/diff/8016/chrome/browser/extensions/...
chrome/browser/extensions/api/extension_action/extension_action_api.cc:34:
#include "ui/gfx/image/image.h"
On 2013/03/04 00:10:46, Matt Giuca wrote:
> Why do you add this #include to a bunch of files? I assume it's an include
what
> you use thing, but it doesn't seem related to this CL.

These were included previously from transitive includes, via extension_service
and app_shortcut_manager. Now that extension_service doesn't include
app_shortcut_manager, these files all fail to compile without this include.

So yeah, it is an IWYU thing: this is why IWYU is important, so poor folks like
me don't get inflicted with this kind of pain when making unrelated changes :)

[benwells, 2013-03-05 02:27:57]

On 2013/03/04 01:30:03, benwells wrote:
> https://codereview.chromium.org/12378037/diff/8016/apps/apps.gypi
> File apps/apps.gypi (right):
> 
> https://codereview.chromium.org/12378037/diff/8016/apps/apps.gypi#newcode48
> apps/apps.gypi:48: '../build/linux/system.gyp:gdk',
> On 2013/03/03 23:57:49, tfarina wrote:
> > why do you need this now?
> 
> good question. it was needed because i am now including profile.h, which
> includes stuff something which needs this.
> 
> I don't think shortcut_manager needs profile.h, so will remove that instead of
> adding this dependency.
> 
>
https://codereview.chromium.org/12378037/diff/8016/apps/shortcut_manager_fact...
> File apps/shortcut_manager_factory.h (right):
> 
>
https://codereview.chromium.org/12378037/diff/8016/apps/shortcut_manager_fact...
> apps/shortcut_manager_factory.h:8: #include "base/memory/singleton.h"
> On 2013/03/03 23:57:49, tfarina wrote:
> > forward declare this:
> > 
> > template <typename T> struct DefaultSingletonTraits; 
> > 
> > and include in the source file instead.
> 
> Done.
> 
>
https://codereview.chromium.org/12378037/diff/8016/apps/shortcut_manager_fact...
> apps/shortcut_manager_factory.h:11: class Profile;
> On 2013/03/03 23:57:49, tfarina wrote:
> > no need to forward declare this, it comes from
profile_keyed_service_factory.h
> 
> Done.
> 
>
https://codereview.chromium.org/12378037/diff/8016/chrome/browser/extensions/...
> File chrome/browser/extensions/api/extension_action/extension_action_api.cc
> (right):
> 
>
https://codereview.chromium.org/12378037/diff/8016/chrome/browser/extensions/...
> chrome/browser/extensions/api/extension_action/extension_action_api.cc:34:
> #include "ui/gfx/image/image.h"
> On 2013/03/04 00:10:46, Matt Giuca wrote:
> > Why do you add this #include to a bunch of files? I assume it's an include
> what
> > you use thing, but it doesn't seem related to this CL.
> 
> These were included previously from transitive includes, via extension_service
> and app_shortcut_manager. Now that extension_service doesn't include
> app_shortcut_manager, these files all fail to compile without this include.
> 
> So yeah, it is an IWYU thing: this is why IWYU is important, so poor folks
like
> me don't get inflicted with this kind of pain when making unrelated changes :)

ping ben

[Ben Goodger (Google), 2013-03-06 17:47:48]

lgtm

[commit-bot: I haz the power, 2013-03-06 21:45:33]

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

[commit-bot: I haz the power, 2013-03-06 21:45:47]

Failed to apply patch for apps/shortcut_manager.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  A         apps/shortcut_manager.cc
  Copied chrome/browser/extensions/app_shortcut_manager.cc ->
apps/shortcut_manager.cc
  patching file apps/shortcut_manager.cc
  Hunk #1 FAILED at 2.
  Hunk #2 succeeded at 34 (offset 1 line).
  Hunk #3 succeeded at 45 (offset 1 line).
  Hunk #4 succeeded at 63 (offset 1 line).
  1 out of 4 hunks FAILED -- saving rejects to file apps/shortcut_manager.cc.rej

Patch:   NR 
chrome/browser/extensions/app_shortcut_manager.cc->apps/shortcut_manager.cc
Index: apps/shortcut_manager.cc
diff --git a/chrome/browser/extensions/app_shortcut_manager.cc
b/apps/shortcut_manager.cc
similarity index 70%
rename from chrome/browser/extensions/app_shortcut_manager.cc
rename to apps/shortcut_manager.cc
index
219894b1155a7f0e89f0448934647a68e793cc0b..267b99555b21817bdeb8bf78380686b1240669ed
100644
--- a/chrome/browser/extensions/app_shortcut_manager.cc
+++ b/apps/shortcut_manager.cc
@@ -2,29 +2,21 @@
 // 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_shortcut_manager.h"
+#include "apps/shortcut_manager.h"
 
 #include "base/bind.h"
-#include "base/command_line.h"
 #include "base/compiler_specific.h"
-#include "base/logging.h"
-#include "base/utf_string_conversions.h"
-#include "chrome/browser/extensions/image_loader.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/web_applications/web_app_ui.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_notification_types.h"
-#include "chrome/common/chrome_switches.h"
-#include "chrome/common/extensions/extension_resource.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
-#include "grit/theme_resources.h"
-#include "skia/ext/image_operations.h"
-#include "ui/base/resource/resource_bundle.h"
 
-namespace extensions {
+using extensions::Extension;
 
-AppShortcutManager::AppShortcutManager(Profile* profile)
+namespace apps {
+
+ShortcutManager::ShortcutManager(Profile* profile)
     : profile_(profile),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_INSTALLED,
@@ -33,9 +25,9 @@ AppShortcutManager::AppShortcutManager(Profile* profile)
                  content::Source<Profile>(profile_));
 }
 
-AppShortcutManager::~AppShortcutManager() {}
+ShortcutManager::~ShortcutManager() {}
 
-void AppShortcutManager::Observe(int type,
+void ShortcutManager::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
   switch (type) {
@@ -44,7 +36,7 @@ void AppShortcutManager::Observe(int type,
       const Extension* extension = content::Details<const Extension>(
           details).ptr();
       if (extension->is_platform_app() &&
-          extension->location() != Manifest::COMPONENT) {
+          extension->location() != extensions::Manifest::COMPONENT) {
         web_app::UpdateShortcutInfoAndIconForApp(*extension, profile_,
           base::Bind(&web_app::UpdateAllShortcuts));
       }
@@ -62,11 +54,11 @@ void AppShortcutManager::Observe(int type,
   }
 }
 
-void AppShortcutManager::DeleteApplicationShortcuts(
+void ShortcutManager::DeleteApplicationShortcuts(
     const Extension* extension) {
   ShellIntegration::ShortcutInfo delete_info =
       web_app::ShortcutInfoForExtensionAndProfile(extension, profile_);
   web_app::DeleteAllShortcuts(delete_info);
 }
 
-}  // namespace extensions
+}  // namespace apps

[commit-bot: I haz the power, 2013-03-07 04:03:51]

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

[commit-bot: I haz the power, 2013-03-07 08:37:45]

Change committed as 186648