kiosk: Fix network check skipped regression.

Fix the the regression by providing a more reasonable default value
for platform apps' default offline_enabled value: Apps default to
being offline enabled unless webview permission is requested.

BUG=349200,350129
TEST=ExtensionManifestOfflineEnabledTest.* and KioskTest.*LaunchAppNetworkDown

Review URL: https://codereview.chromium.org/181233007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@255988 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/login/kiosk_browsertest.cc

@@ -319,6 +319,8 @@ class KioskTest : public OobeBaseTest {
   }
 
   void ReloadKioskApps() {
+    // Remove then add to ensure NOTIFICATION_KIOSK_APPS_LOADED fires.
+    KioskAppManager::Get()->RemoveApp(test_app_id_);
     KioskAppManager::Get()->AddApp(test_app_id_);
   }
 
@@ -330,18 +332,23 @@ class KioskTest : public OobeBaseTest {
   void PrepareAppLaunch() {
     EnableConsumerKioskMode();
 
-    // Start UI, find menu entry for this app and launch it.
+    // Start UI
     content::WindowedNotificationObserver login_signal(
-      chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
-      content::NotificationService::AllSources());
+        chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE,
+        content::NotificationService::AllSources());
     chromeos::WizardController::SkipPostLoginScreensForTesting();
     chromeos::WizardController* wizard_controller =
         chromeos::WizardController::default_controller();
-    CHECK(wizard_controller);
-    wizard_controller->SkipToLoginForTesting(LoginScreenContext());
-    login_signal.Wait();
+    if (wizard_controller) {
+      wizard_controller->SkipToLoginForTesting(LoginScreenContext());
+      login_signal.Wait();
+    } else {
+      // No wizard and running with an existing profile and it should land
+      // on account picker.
+      OobeScreenWaiter(OobeDisplay::SCREEN_ACCOUNT_PICKER).Wait();
+    }
 
-    // Wait for the Kiosk App configuration to reload, then launch the app.
+    // Wait for the Kiosk App configuration to reload.
     content::WindowedNotificationObserver apps_loaded_signal(
         chrome::NOTIFICATION_KIOSK_APPS_LOADED,
         content::NotificationService::AllSources());
@@ -482,6 +489,43 @@ class KioskTest : public OobeBaseTest {
                             true));
   }
 
+  void RunAppLaunchNetworkDownTest() {
+    // Mock network could be configured with owner's password.
+    ScopedCanConfigureNetwork can_configure_network(true, true);
+
+    // Start app launch and wait for network connectivity timeout.
+    StartAppLaunchFromLoginScreen(SimulateNetworkOfflineClosure());
+    OobeScreenWaiter splash_waiter(OobeDisplay::SCREEN_APP_LAUNCH_SPLASH);
+    splash_waiter.Wait();
+    WaitForAppLaunchNetworkTimeout();
+
+    // Configure network link should be visible.
+    JsExpect("$('splash-config-network').hidden == false");
+
+    // Set up fake user manager with an owner for the test.
+    mock_user_manager()->SetActiveUser(kTestOwnerEmail);
+    AppLaunchSigninScreen::SetUserManagerForTesting(mock_user_manager());
+    static_cast<LoginDisplayHostImpl*>(LoginDisplayHostImpl::default_host())
+        ->GetOobeUI()->ShowOobeUI(false);
+
+    // Configure network should bring up lock screen for owner.
+    OobeScreenWaiter lock_screen_waiter(OobeDisplay::SCREEN_ACCOUNT_PICKER);
+    static_cast<AppLaunchSplashScreenActor::Delegate*>(GetAppLaunchController())
+        ->OnConfigureNetwork();
+    lock_screen_waiter.Wait();
+
+    // A network error screen should be shown after authenticating.
+    OobeScreenWaiter error_screen_waiter(OobeDisplay::SCREEN_ERROR_MESSAGE);
+    static_cast<AppLaunchSigninScreen::Delegate*>(GetAppLaunchController())
+        ->OnOwnerSigninSuccess();
+    error_screen_waiter.Wait();
+
+    ASSERT_TRUE(GetAppLaunchController()->showing_network_dialog());
+
+    SimulateNetworkOnline();
+    WaitForAppLaunchSuccess();
+  }
+
   AppLaunchController* GetAppLaunchController() {
     return chromeos::LoginDisplayHostImpl::default_host()
         ->GetAppLaunchController();
@@ -506,41 +550,15 @@ IN_PROC_BROWSER_TEST_F(KioskTest, InstallAndLaunchApp) {
   WaitForAppLaunchSuccess();
 }
 
-IN_PROC_BROWSER_TEST_F(KioskTest, LaunchAppNetworkDown) {
-  // Mock network could be configured with owner's password.
-  ScopedCanConfigureNetwork can_configure_network(true, true);
-
-  // Start app launch and wait for network connectivity timeout.
-  StartAppLaunchFromLoginScreen(SimulateNetworkOfflineClosure());
-  OobeScreenWaiter splash_waiter(OobeDisplay::SCREEN_APP_LAUNCH_SPLASH);
-  splash_waiter.Wait();
-  WaitForAppLaunchNetworkTimeout();
-
-  // Configure network link should be visible.
-  JsExpect("$('splash-config-network').hidden == false");
-
-  // Set up fake user manager with an owner for the test.
-  mock_user_manager()->SetActiveUser(kTestOwnerEmail);
-  AppLaunchSigninScreen::SetUserManagerForTesting(mock_user_manager());
-  static_cast<LoginDisplayHostImpl*>(LoginDisplayHostImpl::default_host())
-      ->GetOobeUI()->ShowOobeUI(false);
-
-  // Configure network should bring up lock screen for owner.
-  OobeScreenWaiter lock_screen_waiter(OobeDisplay::SCREEN_ACCOUNT_PICKER);
-  static_cast<AppLaunchSplashScreenActor::Delegate*>(GetAppLaunchController())
-    ->OnConfigureNetwork();
-  lock_screen_waiter.Wait();
-
-  // A network error screen should be shown after authenticating.
-  OobeScreenWaiter error_screen_waiter(OobeDisplay::SCREEN_ERROR_MESSAGE);
-  static_cast<AppLaunchSigninScreen::Delegate*>(GetAppLaunchController())
-      ->OnOwnerSigninSuccess();
-  error_screen_waiter.Wait();
-
-  ASSERT_TRUE(GetAppLaunchController()->showing_network_dialog());
+IN_PROC_BROWSER_TEST_F(KioskTest, PRE_LaunchAppNetworkDown) {
+  // Tests the network down case for the initial app download and launch.
+  RunAppLaunchNetworkDownTest();
+}
 
-  SimulateNetworkOnline();
-  WaitForAppLaunchSuccess();
+IN_PROC_BROWSER_TEST_F(KioskTest, LaunchAppNetworkDown) {
+  // Tests the network down case for launching an existing app that is
+  // installed in PRE_LaunchAppNetworkDown.
+  RunAppLaunchNetworkDownTest();
 }
 
 IN_PROC_BROWSER_TEST_F(KioskTest, LaunchAppNetworkDownConfigureNotAllowed) {

chrome/common/extensions/docs/templates/articles/manifest/offline_enabled.html

@@ -5,3 +5,18 @@ Whether the app or extension is expected to work offline. When Chrome detects
 that it is offline, apps with this field set to true will be highlighted
 on the New Tab page.
 </p>
+
+<p>
+As of Chrome 35, apps are assumed to be offline enabled and the default value
+of <code>"offline_enabled"</code> is <code>true</code> unless <code>"webview"</code>
+permission is requested. In this case, network connectivity is assumed to be
+required and <code>"offline_enabled"</code> defaults to <code>false</code>.
+</p>
+
+<p>
+The <code>"offline_enabled"</code> value is also used to determine whether a
+network connectivity check will be performed when launching an app in <a href="/apps/manifest/kiosk_enabled">
+ChromeOS kiosk mode</a>. A network connectivity check will be performed when
+apps are not offline enabled, and app launching put on hold until the device
+obtains connectivity to the Internet.
+</p>

chrome/common/extensions/manifest_tests/extension_manifests_offline_unittest.cc

@@ -36,6 +36,9 @@ TEST_F(ExtensionManifestOfflineEnabledTest, OfflineEnabled) {
   scoped_refptr<Extension> extension_5(
       LoadAndExpectSuccess("offline_default_platform_app.json"));
   EXPECT_TRUE(OfflineEnabledInfo::IsOfflineEnabled(extension_5.get()));
+  scoped_refptr<Extension> extension_6(
+      LoadAndExpectSuccess("offline_default_platform_app_with_webview.json"));
+  EXPECT_FALSE(OfflineEnabledInfo::IsOfflineEnabled(extension_6.get()));
 }
 
 }  // namespace extensions

extensions/common/manifest_handlers/offline_enabled_info.cc

@@ -10,6 +10,8 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "extensions/common/manifest_constants.h"
+#include "extensions/common/permissions/api_permission_set.h"
+#include "extensions/common/permissions/permissions_data.h"
 
 namespace extensions {
 
@@ -37,11 +39,17 @@ OfflineEnabledHandler::~OfflineEnabledHandler() {
 
 bool OfflineEnabledHandler::Parse(Extension* extension, base::string16* error) {
   if (!extension->manifest()->HasKey(keys::kOfflineEnabled)) {
-    // Only platform apps default to being enabled offline, and we should only
-    // attempt parsing without a key present if it is a platform app.
+    // Only platform apps are provided with a default offline enabled value.
+    // A platform app is offline enabled unless it requests the webview
+    // permission. That is, offline_enabled is true when there is NO webview
+    // permission requested and false when webview permission is present.
     DCHECK(extension->is_platform_app());
+
+    const bool has_webview_permission =
+        !!PermissionsData::GetInitialAPIPermissions(extension)
+              ->count(APIPermission::kWebView);
     extension->SetManifestData(keys::kOfflineEnabled,
-                               new OfflineEnabledInfo(true));
+                               new OfflineEnabledInfo(!has_webview_permission));
     return true;
   }