Actually check whether extension is in kiosk mode

Add the missing check for actually being in kiosk mode, but keep the
autotest bail to simplify testing of the feature.

BUG=chromium:826568
TEST=Ran autotest, verified that extension can't access edid.

Change-Id: I8f93e29d8d977b0977b3eb597ce2aae263b4e9b9
Reviewed-on: https://chromium-review.googlesource.com/982619Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Felix Ekblom <felixe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548147}

extensions/browser/api/system_display/system_display_api.cc

@@ -25,9 +25,9 @@ namespace extensions {
 
 namespace display = api::system_display;
 
-const char SystemDisplayFunction::kCrosOnlyError[] =
+const char SystemDisplayCrOSRestrictedFunction::kCrosOnlyError[] =
     "Function available only on ChromeOS.";
-const char SystemDisplayFunction::kKioskOnlyError[] =
+const char SystemDisplayCrOSRestrictedFunction::kKioskOnlyError[] =
     "Only kiosk enabled extensions are allowed to use this function.";
 
 namespace {
@@ -148,9 +148,23 @@ bool OverscanTracker::RemoveObserverImpl(content::WebContents* web_contents) {
   return observers_.empty();
 }
 
+bool HasAutotestPrivate(const UIThreadExtensionFunction& function) {
+  return function.extension() &&
+         function.extension()->permissions_data()->HasAPIPermission(
+             APIPermission::kAutoTestPrivate);
+}
+
+#if defined(OS_CHROMEOS)
+// |edid| is available only to Chrome OS kiosk mode applications.
+bool ShouldRestrictEdidInformation(const UIThreadExtensionFunction& function) {
+  return !HasAutotestPrivate(function) &&
+         !KioskModeInfo::IsKioskEnabled(function.extension());
+}
+#endif
+
 }  // namespace
 
-bool SystemDisplayFunction::PreRunValidation(std::string* error) {
+bool SystemDisplayCrOSRestrictedFunction::PreRunValidation(std::string* error) {
   if (!UIThreadExtensionFunction::PreRunValidation(error))
     return false;
 
@@ -170,20 +184,8 @@ bool SystemDisplayFunction::PreRunValidation(std::string* error) {
 #endif
 }
 
-bool SystemDisplayFunction::ShouldRestrictToKioskAndWebUI() {
-  // Allow autotest extension to access for Chrome OS testing.
-  if (extension() && extension()->permissions_data()->HasAPIPermission(
-                         APIPermission::kAutoTestPrivate)) {
-    return false;
-  }
-
-  return true;
-}
-
-bool SystemDisplayGetInfoFunction::PreRunValidation(std::string* error) {
-  // Returns true to not block the method completely when in non-kiosk mode.
-  // Only the |edid| property is conditional to kiosk mode.
-  return true;
+bool SystemDisplayCrOSRestrictedFunction::ShouldRestrictToKioskAndWebUI() {
+  return !HasAutotestPrivate(*this);
 }
 
 ExtensionFunction::ResponseAction SystemDisplayGetInfoFunction::Run() {
@@ -199,11 +201,12 @@ ExtensionFunction::ResponseAction SystemDisplayGetInfoFunction::Run() {
 
 void SystemDisplayGetInfoFunction::Response(
     DisplayInfoProvider::DisplayUnitInfoList all_displays_info) {
-  // |edid| is restricted to kiosk mode.
-  if (ShouldRestrictToKioskAndWebUI()) {
+#if defined(OS_CHROMEOS)
+  if (ShouldRestrictEdidInformation(*this)) {
     for (auto& display_info : all_displays_info)
       display_info.edid.release();
   }
+#endif
   Respond(ArgumentList(display::GetInfo::Results::Create(all_displays_info)));
 }
 

extensions/browser/api/system_display/system_display_api.h

@@ -12,13 +12,13 @@
 
 namespace extensions {
 
-class SystemDisplayFunction : public UIThreadExtensionFunction {
+class SystemDisplayCrOSRestrictedFunction : public UIThreadExtensionFunction {
  public:
   static const char kCrosOnlyError[];
   static const char kKioskOnlyError[];
 
  protected:
-  ~SystemDisplayFunction() override {}
+  ~SystemDisplayCrOSRestrictedFunction() override {}
   bool PreRunValidation(std::string* error) override;
 
   // Returns true if this function should be restricted to kiosk-mode apps and
@@ -26,22 +26,22 @@ class SystemDisplayFunction : public UIThreadExtensionFunction {
   virtual bool ShouldRestrictToKioskAndWebUI();
 };
 
-// GetInfo is a SystemDisplayFunction to limit some of the fields to kiosk mode
-// and web UI. The method itself is not kiosk-mode only.
-class SystemDisplayGetInfoFunction : public SystemDisplayFunction {
+// This function inherits from UIThreadExtensionFunction because, unlike the
+// rest of this API, it's available on all platforms.
+class SystemDisplayGetInfoFunction : public UIThreadExtensionFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.getInfo", SYSTEM_DISPLAY_GETINFO);
 
  protected:
   ~SystemDisplayGetInfoFunction() override {}
 
-  bool PreRunValidation(std::string* error) override;
   ResponseAction Run() override;
 
   void Response(DisplayInfoProvider::DisplayUnitInfoList all_displays_info);
 };
 
-class SystemDisplayGetDisplayLayoutFunction : public SystemDisplayFunction {
+class SystemDisplayGetDisplayLayoutFunction
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.getDisplayLayout",
                              SYSTEM_DISPLAY_GETDISPLAYLAYOUT);
@@ -54,7 +54,8 @@ class SystemDisplayGetDisplayLayoutFunction : public SystemDisplayFunction {
   void Response(DisplayInfoProvider::DisplayLayoutList display_layout);
 };
 
-class SystemDisplaySetDisplayPropertiesFunction : public SystemDisplayFunction {
+class SystemDisplaySetDisplayPropertiesFunction
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.setDisplayProperties",
                              SYSTEM_DISPLAY_SETDISPLAYPROPERTIES);
@@ -66,7 +67,8 @@ class SystemDisplaySetDisplayPropertiesFunction : public SystemDisplayFunction {
   void Response(base::Optional<std::string> error);
 };
 
-class SystemDisplaySetDisplayLayoutFunction : public SystemDisplayFunction {
+class SystemDisplaySetDisplayLayoutFunction
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.setDisplayLayout",
                              SYSTEM_DISPLAY_SETDISPLAYLAYOUT);
@@ -78,7 +80,8 @@ class SystemDisplaySetDisplayLayoutFunction : public SystemDisplayFunction {
   void Response(base::Optional<std::string> error);
 };
 
-class SystemDisplayEnableUnifiedDesktopFunction : public SystemDisplayFunction {
+class SystemDisplayEnableUnifiedDesktopFunction
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.enableUnifiedDesktop",
                              SYSTEM_DISPLAY_ENABLEUNIFIEDDESKTOP);
@@ -89,7 +92,7 @@ class SystemDisplayEnableUnifiedDesktopFunction : public SystemDisplayFunction {
 };
 
 class SystemDisplayOverscanCalibrationStartFunction
-    : public SystemDisplayFunction {
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.overscanCalibrationStart",
                              SYSTEM_DISPLAY_OVERSCANCALIBRATIONSTART);
@@ -100,7 +103,7 @@ class SystemDisplayOverscanCalibrationStartFunction
 };
 
 class SystemDisplayOverscanCalibrationAdjustFunction
-    : public SystemDisplayFunction {
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.overscanCalibrationAdjust",
                              SYSTEM_DISPLAY_OVERSCANCALIBRATIONADJUST);
@@ -111,7 +114,7 @@ class SystemDisplayOverscanCalibrationAdjustFunction
 };
 
 class SystemDisplayOverscanCalibrationResetFunction
-    : public SystemDisplayFunction {
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.overscanCalibrationReset",
                              SYSTEM_DISPLAY_OVERSCANCALIBRATIONRESET);
@@ -122,7 +125,7 @@ class SystemDisplayOverscanCalibrationResetFunction
 };
 
 class SystemDisplayOverscanCalibrationCompleteFunction
-    : public SystemDisplayFunction {
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.overscanCalibrationComplete",
                              SYSTEM_DISPLAY_OVERSCANCALIBRATIONCOMPLETE);
@@ -133,7 +136,7 @@ class SystemDisplayOverscanCalibrationCompleteFunction
 };
 
 class SystemDisplayShowNativeTouchCalibrationFunction
-    : public SystemDisplayFunction {
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.showNativeTouchCalibration",
                              SYSTEM_DISPLAY_SHOWNATIVETOUCHCALIBRATION);
@@ -146,7 +149,7 @@ class SystemDisplayShowNativeTouchCalibrationFunction
 };
 
 class SystemDisplayStartCustomTouchCalibrationFunction
-    : public SystemDisplayFunction {
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.startCustomTouchCalibration",
                              SYSTEM_DISPLAY_STARTCUSTOMTOUCHCALIBRATION);
@@ -157,7 +160,7 @@ class SystemDisplayStartCustomTouchCalibrationFunction
 };
 
 class SystemDisplayCompleteCustomTouchCalibrationFunction
-    : public SystemDisplayFunction {
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.completeCustomTouchCalibration",
                              SYSTEM_DISPLAY_COMPLETECUSTOMTOUCHCALIBRATION);
@@ -168,7 +171,7 @@ class SystemDisplayCompleteCustomTouchCalibrationFunction
 };
 
 class SystemDisplayClearTouchCalibrationFunction
-    : public SystemDisplayFunction {
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.clearTouchCalibration",
                              SYSTEM_DISPLAY_CLEARTOUCHCALIBRATION);
@@ -178,7 +181,8 @@ class SystemDisplayClearTouchCalibrationFunction
   ResponseAction Run() override;
 };
 
-class SystemDisplaySetMirrorModeFunction : public SystemDisplayFunction {
+class SystemDisplaySetMirrorModeFunction
+    : public SystemDisplayCrOSRestrictedFunction {
  public:
   DECLARE_EXTENSION_FUNCTION("system.display.setMirrorMode",
                              SYSTEM_DISPLAY_SETMIRRORMODE);

extensions/browser/api/system_display/system_display_apitest.cc

@@ -247,7 +247,7 @@ IN_PROC_BROWSER_TEST_F(SystemDisplayApiTest, SetDisplay) {
   set_info_function->set_has_callback(true);
 
   EXPECT_EQ(
-      SystemDisplayFunction::kCrosOnlyError,
+      SystemDisplayCrOSRestrictedFunction::kCrosOnlyError,
       api_test_utils::RunFunctionAndReturnError(
           set_info_function.get(), "[\"display_id\", {}]", browser_context()));
 
@@ -297,7 +297,7 @@ IN_PROC_BROWSER_TEST_F(SystemDisplayApiTest, SetDisplayNotKioskEnabled) {
   set_info_function->set_has_callback(true);
 
   EXPECT_EQ(
-      SystemDisplayFunction::kKioskOnlyError,
+      SystemDisplayCrOSRestrictedFunction::kKioskOnlyError,
       api_test_utils::RunFunctionAndReturnError(
           set_info_function.get(), "[\"display_id\", {}]", browser_context()));