No longer start BG mode until a BackgroundContents is loaded

chrome/browser/extensions/app_background_page_apitest.cc

 // Copyright (c) 2012 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 "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/background/background_contents_service.h"
 #include "chrome/browser/background/background_contents_service_factory.h"
+#include "chrome/browser/background/background_mode_manager.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_service.h"
 #include "content/test/test_notification_tracker.h"
 #include "net/base/mock_host_resolver.h"
 
 class AppBackgroundPageApiTest : public ExtensionApiTest {
  public:
   void SetUpCommandLine(CommandLine* command_line) OVERRIDE {
     ExtensionApiTest::SetUpCommandLine(command_line);
     command_line->AppendSwitch(switches::kDisablePopupBlocking);
     command_line->AppendSwitch(switches::kAllowHTTPBackgroundPage);
   }
@@ skipping 10 matching lines @@
                                              app_manifest.size());
     if (bytes_written != static_cast<int>(app_manifest.size())) {
       LOG(ERROR) << "Unable to write complete manifest to file. Return code="
                  << bytes_written;
       return false;
     }
     *app_dir = app_dir_.path();
     return true;
   }
 
+  // Blocks waiting for BackgroundModeManager to transition background mode
+  // to the expected state.
+  class BackgroundModeWatcher : public content::NotificationObserver {
+   public:
+    explicit BackgroundModeWatcher(bool expected) : expected_(expected) {
+      registrar_.Add(
+          this, chrome::NOTIFICATION_BACKGROUND_MODE_CHANGED,

[Mihai Parparita -not on Chrome, 2012/05/03 19:32:35]

Does this end up firing more than once, before it gets to the expected value, or
can you just use WindowedNotificationObserverWithDetails?

+          content::NotificationService::AllSources());
+    }
+
+    void WaitForChange() {
+      ui_test_utils::RunMessageLoop();
+    }
+
+    virtual void Observe(int type,
+                         const content::NotificationSource& source,
+                         const content::NotificationDetails& details) OVERRIDE {
+      // If BackgroundMode is finally in the expected state, exit.
+      if (expected_ == *content::Details<bool>(details).ptr())
+        MessageLoopForUI::current()->Quit();
+    }
+
+   private:
+    content::NotificationRegistrar registrar_;
+    // The background mode value we are waiting for.
+    bool expected_;
+  };
+
+  bool WaitForBackgroundMode(bool expected_background_mode) {
+    BackgroundModeManager* manager =
+        g_browser_process->background_mode_manager();
+    // If background mode is disabled on this platform (e.g. cros), then skip
+    // this check.
+    if (!manager || !manager->IsBackgroundModePrefEnabled()) {
+      DLOG(WARNING) << "Skipping check - background mode disabled";
+      return true;
+    }
+    if (manager->IsBackgroundModeActiveForTest() == expected_background_mode)
+      return true;
+
+    // We are not currently in the expected state - wait for the state to
+    // change.
+    BackgroundModeWatcher watcher(expected_background_mode);
+    watcher.WaitForChange();
+    return manager->IsBackgroundModeActiveForTest() == expected_background_mode;
+  }
+
  private:
   ScopedTempDir app_dir_;
 };
 
 // Disable on Mac only.  http://crbug.com/95139
 #if defined(OS_MACOSX)
 #define MAYBE_Basic DISABLED_Basic
 #else
 #define MAYBE_Basic Basic
 #endif
@@ skipping 15 matching lines @@
       "      \"web_url\": \"http://a.com:%d/\""
       "    }"
       "  },"
       "  \"permissions\": [\"background\"]"
       "}",
       test_server()->host_port_pair().port());
 
   FilePath app_dir;
   ASSERT_TRUE(CreateApp(app_manifest, &app_dir));
   ASSERT_TRUE(LoadExtension(app_dir));
+  // Background mode should not be active until a background page is created.
+  ASSERT_TRUE(WaitForBackgroundMode(false));
   ASSERT_TRUE(RunExtensionTest("app_background_page/basic")) << message_;
+  // The test closes the background contents, so we should fall back to no
+  // background mode at the end.
+  ASSERT_TRUE(WaitForBackgroundMode(false));
 }
 
 // Crashy, http://crbug.com/69215.
 IN_PROC_BROWSER_TEST_F(AppBackgroundPageApiTest, DISABLED_LacksPermission) {
   host_resolver()->AddRule("a.com", "127.0.0.1");
   ASSERT_TRUE(StartTestServer());
 
   std::string app_manifest = base::StringPrintf(
       "{"
       "  \"name\": \"App\","
       "  \"version\": \"0.1\","
       "  \"manifest_version\": 2,"
       "  \"app\": {"
       "    \"urls\": ["
       "      \"http://a.com/\""
       "    ],"
       "    \"launch\": {"
       "      \"web_url\": \"http://a.com:%d/\""
       "    }"
       "  }"
       "}",
       test_server()->host_port_pair().port());
 
   FilePath app_dir;
   ASSERT_TRUE(CreateApp(app_manifest, &app_dir));
   ASSERT_TRUE(LoadExtension(app_dir));
   ASSERT_TRUE(RunExtensionTest("app_background_page/lacks_permission"))
       << message_;
+  ASSERT_TRUE(WaitForBackgroundMode(false));
 }
 
 IN_PROC_BROWSER_TEST_F(AppBackgroundPageApiTest, ManifestBackgroundPage) {
   host_resolver()->AddRule("a.com", "127.0.0.1");
   ASSERT_TRUE(StartTestServer());
 
   std::string app_manifest = base::StringPrintf(
       "{"
       "  \"name\": \"App\","
       "  \"version\": \"0.1\","
       "  \"manifest_version\": 2,"
       "  \"app\": {"
       "    \"urls\": ["
       "      \"http://a.com/\""
       "    ],"
       "    \"launch\": {"
       "      \"web_url\": \"http://a.com:%d/\""
       "    }"
       "  },"
       "  \"permissions\": [\"background\"],"
       "  \"background\": {"
       "    \"page\": \"http://a.com:%d/test.html\""
       "  }"
       "}",
       test_server()->host_port_pair().port(),
       test_server()->host_port_pair().port());
 
   FilePath app_dir;
   ASSERT_TRUE(CreateApp(app_manifest, &app_dir));
+  // Background mode should not be active now because no background app was
+  // loaded.
   ASSERT_TRUE(LoadExtension(app_dir));
+  // Background mode be active now because a background page was created when
+  // the app was loaded.
+  ASSERT_TRUE(WaitForBackgroundMode(true));
 
   const Extension* extension = GetSingleLoadedExtension();
   ASSERT_TRUE(
       BackgroundContentsServiceFactory::GetForProfile(browser()->profile())->
           GetAppBackgroundContents(ASCIIToUTF16(extension->id())));
 }
 
 IN_PROC_BROWSER_TEST_F(AppBackgroundPageApiTest, NoJsBackgroundPage) {
   // Make sure that no BackgroundContentses get deleted (a signal that repeated
   // window.open calls recreate instances, instead of being no-ops).
@@ skipping 165 matching lines @@
       "  \"permissions\": [\"background\"]"
       "}",
       test_server()->host_port_pair().port(),
       test_server()->host_port_pair().port());
 
   FilePath app_dir;
   ASSERT_TRUE(CreateApp(app_manifest, &app_dir));
   ASSERT_TRUE(LoadExtension(app_dir));
   ASSERT_TRUE(RunExtensionTest("app_background_page/bg_open")) << message_;
 }
+
+IN_PROC_BROWSER_TEST_F(AppBackgroundPageApiTest, OpenThenClose) {
+  host_resolver()->AddRule("a.com", "127.0.0.1");
+  ASSERT_TRUE(StartTestServer());
+
+  std::string app_manifest = base::StringPrintf(
+      "{"
+      "  \"name\": \"App\","
+      "  \"version\": \"0.1\","
+      "  \"manifest_version\": 2,"
+      "  \"app\": {"
+      "    \"urls\": ["
+      "      \"http://a.com/\""
+      "    ],"
+      "    \"launch\": {"
+      "      \"web_url\": \"http://a.com:%d/\""
+      "    }"
+      "  },"
+      "  \"permissions\": [\"background\"]"
+      "}",
+      test_server()->host_port_pair().port());
+
+  FilePath app_dir;
+  ASSERT_TRUE(CreateApp(app_manifest, &app_dir));
+  ASSERT_TRUE(LoadExtension(app_dir));
+  // There isn't a background page loaded initially.
+  const Extension* extension = GetSingleLoadedExtension();
+  ASSERT_FALSE(
+      BackgroundContentsServiceFactory::GetForProfile(browser()->profile())->
+          GetAppBackgroundContents(ASCIIToUTF16(extension->id())));
+  // Background mode should not be active until a background page is created.
+  ASSERT_TRUE(WaitForBackgroundMode(false));
+  ASSERT_TRUE(RunExtensionTest("app_background_page/basic_open")) << message_;
+  // Background mode should be active now because a background page was created.
+  ASSERT_TRUE(WaitForBackgroundMode(true));
+  ASSERT_TRUE(
+      BackgroundContentsServiceFactory::GetForProfile(browser()->profile())->
+          GetAppBackgroundContents(ASCIIToUTF16(extension->id())));
+  // Now close the BackgroundContents.
+  ASSERT_TRUE(RunExtensionTest("app_background_page/basic_close")) << message_;
+  // Background mode should no longer be active.
+  ASSERT_TRUE(WaitForBackgroundMode(false));
+  ASSERT_FALSE(
+      BackgroundContentsServiceFactory::GetForProfile(browser()->profile())->
+          GetAppBackgroundContents(ASCIIToUTF16(extension->id())));
+

[Mihai Parparita -not on Chrome, 2012/05/03 19:32:35]

Nit: extra newline.

+}