app_list: Drive app integration.

chrome/browser/apps/drive/drive_app_provider.h

Index: chrome/browser/apps/drive/drive_app_provider.h
diff --git a/chrome/browser/apps/drive/drive_app_provider.h b/chrome/browser/apps/drive/drive_app_provider.h
new file mode 100644
index 0000000000000000000000000000000000000000..d9c016ceac5596299d80cce6c5c2570d11d771f4
--- /dev/null
+++ b/chrome/browser/apps/drive/drive_app_provider.h
@@ -0,0 +1,116 @@
+// Copyright 2014 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.
+
+#ifndef CHROME_BROWSER_APPS_DRIVE_DRIVE_APP_PROVIDER_H_
+#define CHROME_BROWSER_APPS_DRIVE_DRIVE_APP_PROVIDER_H_
+
+#include <set>
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "base/memory/weak_ptr.h"
+#include "chrome/browser/drive/drive_app_registry_observer.h"
+#include "chrome/browser/extensions/install_observer.h"
+
+namespace app_list {
+class AppListModel;

[benwells, 2014/06/06 00:31:04]

This dependency is not good. My questions about MigrateModelSettings were for me
to understand why this is needed.

To remove the dependency, we could introduce some sort of delegate that the app
list model listens to.

But I'm wondering if we need to (see below).

+}
+
+namespace drive {
+struct DriveAppInfo;
+}
+
+class BrowserContextKeyedServiceFactory;
+class DriveAppConverter;
+class DriveAppMapping;
+class DriveServiceBridge;
+class ExtensionService;
+class Profile;
+
+// DriveAppProvider is the integration point for Drive apps. It ensures each
+// Drive app has a corresponding Chrome app in the extension system. If there
+// is no matching Chrome app, a local URL app would be created. The class
+// processes app changes from both DriveAppRegistry and extension system to
+// keep the two in sync.
+class DriveAppProvider : public drive::DriveAppRegistryObserver,
+                         public extensions::InstallObserver {
+ public:
+  DriveAppProvider(Profile* profile, app_list::AppListModel* model);
+  virtual ~DriveAppProvider();
+
+  // Appends PKS factories this class depends on.
+  static void AppendDependsOnFactories(
+      std::set<BrowserContextKeyedServiceFactory*>* factories);
+
+  void SetDriveServiceBridgeForTest(scoped_ptr<DriveServiceBridge> test_bridge);
+
+ private:
+  friend class DriveAppProviderTest;
+
+  typedef std::set<std::string> IdSet;
+  typedef std::vector<drive::DriveAppInfo> DriveAppInfos;
+
+  // Migrate launcher model data of |old_chrome_app_id| to |new_chrome_app_id|
+  // when a Drive app is mapped to a different app ID. This happens when
+  // the matching Chrome app is installed by user. The placeholder URL app will
+  // be auto uninstalled and this function is called to migrate its model data
+  // (folder/position info) to the Chrome app. Another possible edge case is
+  // the Drive app changes its create_url and a new placeholder URL app will be
+  // used.
+  void MigrateModelSettings(const std::string& drive_app_id,

[benwells, 2014/06/06 00:31:04]

Has this behavior come from the UX team? I couldn't find it described in either
of the design docs.

I think it will be confusing as described. If a user installs an app it should
be treated like any other new app, and be added to the top level in the folder.

This also means there is no need for the dependency back onto app list.

[xiyuan, 2014/06/06 01:32:01]

The behavior is not defined by UX. I guess I put it this way is partly because
my first proposal is to have typed items and changing the item type dynamically.
I stuck with that impression and feel that we should keep the model settings
when that needs to happen.

I agree this could be confusing. I will remove this and get rid of AppListModel
dependency. We can revisit this when UX requests it.  

+                            const std::string& old_chrome_app_id,
+                            const std::string& new_chrome_app_id);
+
+  // Updates drive app id to chrome app id mapping. If the drive app has

[benwells, 2014/06/06 00:31:04]

Nit: has -> was.

+  // not mapped, add a mapping to |new_app|'s id. Do nothing if it is mapped to
+  // |new_app|'s id already. Things become complicated when there is an existing
+  // mapping. If the existing one is an URL app, update the mapping to |new_app|
+  // and call MigrateModelSettings to migrate model data. If the existing one
+  // is an Chrome app and |new_app| is an URL app, the existing mapping is
+  // preferred and |new_app| will auto uninstalled. Otherwise, if |new_app| is

[benwells, 2014/06/06 00:31:04]

Again I'm not sure of this behaviour. I think the only way it can happen is if
the user manually adds a URL app. In that case we shouldn't auto uninstall it.

[xiyuan, 2014/06/06 01:32:01]

Agree. The CL does too much. I will change it to keep |new_app| in this case. 

+  // a Chrome app, the mapping is updated to use |new_app| but existing Chrome
+  // app is kept.
+  void UpdateMappingAndExtensionSystem(const std::string& drive_app_id,
+                                       const extensions::Extension* new_app);
+
+  // Deferred processing of relevant extension installed message.
+  void ProcessDeferredOnExtensionInstalled(const std::string drive_app_id,
+                                           const std::string chrome_app_id);
+
+  void SchedulePendingConverters();
+  void OnLocalAppConverted(const DriveAppConverter* converter, bool success);
+
+  bool IsDriveAppUpToDate(const drive::DriveAppInfo& drive_app) const;
+
+  void AddOrUpdateDriveApp(const drive::DriveAppInfo& drive_app);
+  void RemoveDriveApp(const std::string& drive_app_id);
+
+  // drive::DriveAppRegistryObserver overrides:
+  virtual void OnDriveAppRegistryUpdated() OVERRIDE;
+
+  // extensions::InstallObserver overrides:
+  virtual void OnExtensionInstalled(
+      const extensions::Extension* extension) OVERRIDE;
+  virtual void OnExtensionUninstalled(
+      const extensions::Extension* extension) OVERRIDE;
+
+  Profile* profile_;
+  app_list::AppListModel* model_;  // Not owned. Must out live this class.
+
+  scoped_ptr<DriveServiceBridge> service_bridge_;
+  scoped_ptr<DriveAppMapping> mapping_;
+  DriveAppInfos drive_apps_;
+
+  // Tracks the pending web app convertions.
+  ScopedVector<DriveAppConverter> pending_converters_;
+
+  base::WeakPtrFactory<DriveAppProvider> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(DriveAppProvider);
+};
+
+#endif  // CHROME_BROWSER_APPS_DRIVE_DRIVE_APP_PROVIDER_H_