Perform install tasks for newly installed or upgraded component apps/extensions.

chrome/browser/extensions/platform_app_browsertest.cc

Index: chrome/browser/extensions/platform_app_browsertest.cc
diff --git a/chrome/browser/extensions/platform_app_browsertest.cc b/chrome/browser/extensions/platform_app_browsertest.cc
index 1b9d46d1afa9908aee306c636d6a0f9ae0c6f096..bc53fcf2c966e3d643e01b308796bae5b06ce5e0 100644
--- a/chrome/browser/extensions/platform_app_browsertest.cc
+++ b/chrome/browser/extensions/platform_app_browsertest.cc
@@ -20,6 +20,7 @@
 #include "chrome/browser/extensions/platform_app_browsertest_util.h"
 #include "chrome/browser/extensions/platform_app_launcher.h"
 #include "chrome/browser/extensions/shell_window_registry.h"
+#include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/tab_contents/render_view_context_menu.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
@@ -823,4 +824,87 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, ReloadRelaunches) {
   ASSERT_TRUE(GetFirstShellWindow());
 }
 
+// Component App Test 1 of 3: ensure that the initial load of a component
+// extension utilising a background page (i.e. a v2 platform app) has its

[Yoyo Zhou, 2012/11/27 23:54:18]

I think you meant e.g. not i.e.

[tapted, 2012/11/28 06:21:14]

Done.

+// background page run and is launchable. Waits for the Launched response from
+// the script resource in the opened shell window.
+IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
+                       PRE_PRE_ComponentAppBackgroundPage) {
+  ExtensionTestMessageListener launched_listener("Launched", false);

[Yoyo Zhou, 2012/11/27 23:54:18]

Declare this closer to the point where it might start being used (say, at 843).

[tapted, 2012/11/28 06:21:14]

Done.

+  content::WindowedNotificationObserver app_loaded_observer(

[Yoyo Zhou, 2012/11/27 23:54:18]

What is the purpose of this observer? If according to the comment at 858 it's
for the app being installed, why not observe NOTIFICATION_EXTENSION_INSTALLED?
It seems like the background page ought to be loaded regardless of whether the
app is installed.

[tapted, 2012/11/28 06:21:14]

I think NOTIFICATION_EXTENSION_INSTALLED comes out too early for this case.
NOTIFICATION_EXTENSION_INSTALLED happens when the extension is added to
ExtensionService, just before a call to AddExtension, which then calls
NotifyExtensionLoaded, which emits NOTIFICATION_EXTENSION_LOADED to be picked up
by the EventRouter, and only at that point would any pre-registered lazy
listeners get registered, thus making it safe to call OpenApplication (since the
App's OnLaunched event would now be registered).

In the 'old' scenario, component extensions are never installed, so that code
path (via NOTIFICATION_EXTENSION_LOADED when ComponentLoader called AddExtension
directly) meant that there were no lazy listeners, OnLaunched never registered,
and the app could not start.

Since this CL would add installation, we need to wait for the background page to
be run (now, in response to a PostTask to call DispatchOnInstalledEvent() in
event_router.cc [triggered by NOTIFICATION_EXTENSION_INSTALLED] winding up in
EventRouter::MaybeLoadLazyBackgroundPageToDispatchEvent(..) ). Once the
background page has run (and its OnLaunched listener registered)
NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME is sent out. So, waiting on that allows
the test to ensure the OnLaunched event is registered before trying to launch
the app.

A lot of the platform apps use this technique (encapsulated in
PlatformAppBrowserTest::LoadAndLaunchPlatformApp(..) 
http://code.google.com/searchframe#OAMlx_jo-ck/src/chrome/browser/extensions/...
). However, that utility function loads the extension as a "regular" extension,
from a directory. However, we need to test loading the extension as a component
extension, so I decomposed it.

I'll add a comment describing this better.

+      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+      content::NotificationService::AllSources());
+
+  const Extension* extension = LoadExtensionAsComponent(
+      test_data_dir_.AppendASCII("platform_apps").AppendASCII("component"));
+  ASSERT_TRUE(extension);
+
+  app_loaded_observer.Wait();
+
+  application_launch::OpenApplication(application_launch::LaunchParams(
+          browser()->profile(), extension, extension_misc::LAUNCH_NONE,
+          NEW_WINDOW));
+
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+}
+
+// Component App Test 2 of 3: ensure an installed component app can be launched
+// on a subsequent browser start, without requiring any install/upgrade logic
+// to be run, then perform setup for step 3.
+IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
+                       PRE_ComponentAppBackgroundPage) {
+  ExtensionTestMessageListener launched_listener("Launched", false);
+
+  // Note: no app_loaded_observer: since component app is now installed,
+  // re-adding it in the same profile should not cause it to be re-installed.

[Yoyo Zhou, 2012/11/27 23:54:18]

Is this tested?

[tapted, 2012/11/28 06:21:14]

Initially I had thought checking for an event to _not_ happen would be too
flaky, but I think I have a good solution in the next patchset - let me know
what you think.

Link:
https://codereview.chromium.org/11275069/diff/13038/chrome/browser/extensions...

Essentially, an observer for chrome::NOTIFICATION_EXTENSION_INSTALLED that just
sets a flag when observed. Test parts 1 and 3 assert the flag gets set before
launch. Test part 2 asserts the flag is not set.

+  const Extension* extension = LoadExtensionAsComponent(
+      test_data_dir_.AppendASCII("platform_apps").AppendASCII("component"));
+  ASSERT_TRUE(extension);
+
+  application_launch::OpenApplication(application_launch::LaunchParams(
+          browser()->profile(), extension, extension_misc::LAUNCH_NONE,
+          NEW_WINDOW));
+
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+
+  ExtensionPrefs* extension_prefs =
+      extensions::ExtensionSystem::Get(browser()->profile())->
+      extension_service()->extension_prefs();
+  (void)extension_prefs;

[Yoyo Zhou, 2012/11/27 23:54:18]

What is this for?

[tapted, 2012/11/28 06:21:14]

Ah, sorry - that was a stray from experimenting with other ways to simulate the
"downgrade". Removed.

+
+  // Clear the registered events to ensure they are updated.

[Yoyo Zhou, 2012/11/27 23:54:18]

This comment should be at 870.

[tapted, 2012/11/28 06:21:14]

Done.

+  extension_prefs->SetRegisteredEvents(extension->id(),
+                                       std::set<std::string>());
+
+  // Simulate a "downgrade" from version 2 in the test manifest.json to 1.
+  const base::StringValue old_version("1");
+  std::string pref_path("extensions.settings.");
+  pref_path += extension->id();
+  pref_path += ".manifest.version";
+  extension_prefs->pref_service()->RegisterStringPref(
+      pref_path.c_str(), std::string(), PrefServiceBase::UNSYNCABLE_PREF);
+  extension_prefs->pref_service()->Set(pref_path.c_str(), old_version);
+}
+
+// Component App Test 3 of 3: simulate a component extension upgrade that
+// re-adds the OnLaunched event, and allows the app to be launched.
+IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, ComponentAppBackgroundPage) {
+  ExtensionTestMessageListener launched_listener("Launched", false);
+
+  // Since we are forcing an upgrade, we need to wait for the load again.
+  content::WindowedNotificationObserver app_loaded_observer(
+      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+      content::NotificationService::AllSources());
+  const Extension* extension = LoadExtensionAsComponent(
+      test_data_dir_.AppendASCII("platform_apps").AppendASCII("component"));
+  ASSERT_TRUE(extension);
+  app_loaded_observer.Wait();
+
+  application_launch::OpenApplication(application_launch::LaunchParams(
+          browser()->profile(), extension, extension_misc::LAUNCH_NONE,
+          NEW_WINDOW));
+
+  ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
+}
+
 }  // namespace extensions