Restrict use of two app-launching command line flags

apps/load_and_launch_browsertest.cc

 // 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.
 
 // Tests for the --load-and-launch-app switch.
 // The two cases are when chrome is running and another process uses the switch
 // and when chrome is started from scratch.
 
 #include "apps/switches.h"
 #include "base/process/launch.h"
 #include "base/test/test_timeouts.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/simple_message_box_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/test_launcher.h"
+#include "extensions/browser/extension_registry.h"
 #include "extensions/test/extension_test_message_listener.h"
 
 using extensions::PlatformAppBrowserTest;
 
 namespace apps {
 
 namespace {
 
 const char* kSwitchesToCopy[] = {
     switches::kUserDataDir,
@@ skipping 77 matching lines @@
 
   ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
   int exit_code;
   ASSERT_TRUE(process.WaitForExitWithTimeout(TestTimeouts::action_timeout(),
                                              &exit_code));
   ASSERT_EQ(0, exit_code);
 }
 
 namespace {
 
-// TestFixture that appends --load-and-launch-app before calling BrowserMain.
+// TestFixture that appends --load-and-launch-app with an app before calling
+// BrowserMain.
 class PlatformAppLoadAndLaunchBrowserTest : public PlatformAppBrowserTest {

[msw, 2016/07/21 17:13:49]

optional nit: LoadAndLaunchPlatformAppBrowserTest

[proberge, 2016/07/25 20:40:53]

Done.

  protected:
   PlatformAppLoadAndLaunchBrowserTest() {}
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     PlatformAppBrowserTest::SetUpCommandLine(command_line);
     app_path_ = test_data_dir_

[msw, 2016/07/21 17:13:50]

optional nit: not used elsewhere; make this function-local.

[proberge, 2016/07/25 20:40:53]

Done.

         .AppendASCII("platform_apps")
         .AppendASCII("minimal");
     command_line->AppendSwitchNative(apps::kLoadAndLaunchApp,
                                      app_path_.value());
   }
 
   void LoadAndLaunchApp() {
     ExtensionTestMessageListener launched_listener("Launched", false);
     ASSERT_TRUE(launched_listener.WaitUntilSatisfied());
 
     // Start an actual browser because we can't shut down with just an app
     // window.
     CreateBrowser(ProfileManager::GetActiveUserProfile());
   }
 
  private:
   base::FilePath app_path_;
 
   DISALLOW_COPY_AND_ASSIGN(PlatformAppLoadAndLaunchBrowserTest);
 };
 
+// TestFixture that appends --load-and-launch-app with an extension before
+// calling BrowserMain.
+class LoadAndLaunchExtensionBrowserTest : public PlatformAppBrowserTest {
+ protected:
+  LoadAndLaunchExtensionBrowserTest() {}
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    PlatformAppBrowserTest::SetUpCommandLine(command_line);
+    app_path_ = test_data_dir_.AppendASCII("good")

[msw, 2016/07/21 17:13:50]

nit: not used elsewhere; make this function-local.

[proberge, 2016/07/25 20:40:53]

Done.

+                    .AppendASCII("Extensions")
+                    .AppendASCII("behllobkkfkfnphdnhnkndlbkcpglgmj")

[msw, 2016/07/21 17:13:50]

nit: cache extension id string (file-local static?) for use in test fixture.

[proberge, 2016/07/25 20:40:53]

Done.

+                    .AppendASCII("1.0.0.0");
+    command_line->AppendSwitchNative(apps::kLoadAndLaunchApp,
+                                     app_path_.value());
+  }
+
+  void SetUpInProcessBrowserTestFixture() override {
+    PlatformAppBrowserTest::SetUpInProcessBrowserTestFixture();
+
+    // Skip showing the error message box in order to avoid freezing the main

[msw, 2016/07/21 17:13:50]

optional nit: -'in order' for a one-liner

[proberge, 2016/07/25 20:40:53]

Done.

+    // thread.
+    chrome::internal::g_should_skip_message_box_for_test = true;
+  }
+
+ private:
+  base::FilePath app_path_;
+
+  DISALLOW_COPY_AND_ASSIGN(LoadAndLaunchExtensionBrowserTest);
+};
+
 }  // namespace
 
 
 // TODO(jackhou): Make this test not flaky on Vista or Linux Aura. See

[msw, 2016/07/21 17:13:50]

Would you mind trying to re-enable this test in a followup? We don't support
vista anymore and there aren't bots for it; and this likely meant to disable on
linux desktop, but it also disables cros; all linux is aura now. Pinged the bug.

[proberge, 2016/07/25 20:40:53]

Sure, I'll try in a follow-up CL.

 // http://crbug.com/176897
 #if defined(OS_WIN) || (defined(OS_LINUX) && defined(USE_AURA))
 #define MAYBE_LoadAndLaunchAppChromeNotRunning \
         DISABLED_LoadAndLaunchAppChromeNotRunning
 #else
 #define MAYBE_LoadAndLaunchAppChromeNotRunning \
         LoadAndLaunchAppChromeNotRunning
 #endif
 
 // Case where Chrome is not running.
 IN_PROC_BROWSER_TEST_F(PlatformAppLoadAndLaunchBrowserTest,
                        MAYBE_LoadAndLaunchAppChromeNotRunning) {
   LoadAndLaunchApp();

[msw, 2016/07/21 17:13:50]

optional nit: check that the app actually loaded?

[proberge, 2016/07/25 20:40:53]

Will do once the test is re-enabled.

 }
 
+IN_PROC_BROWSER_TEST_F(LoadAndLaunchExtensionBrowserTest,
+                       LoadAndLaunchExtension) {
+  const std::vector<base::string16>* errors =
+      ExtensionErrorReporter::GetInstance()->GetErrors();
+
+  // Expect kUnpackedExtensionInsteadOfAppError.

[msw, 2016/07/21 17:13:49]

nit: check for this value?

[proberge, 2016/07/25 20:40:53]

Done.

+  EXPECT_EQ(1u, errors->size());

[msw, 2016/07/21 17:13:50]

Does this ever run on official builds? If so, it'll fail, right?

[proberge, 2016/07/25 20:40:53]

Good catch! Thanks

+
+  extensions::ExtensionRegistry* registry =
+      extensions::ExtensionRegistry::Get(profile());
+  EXPECT_EQ(nullptr, registry->GetExtensionById(
+                         "behllobkkfkfnphdnhnkndlbkcpglgmj",
+                         extensions::ExtensionRegistry::EVERYTHING));
+}
+
 }  // namespace apps