Allow component IME extension use app.window and add 'ime' window type for app window

chrome/browser/extensions/api/app_window/app_window_api.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 "chrome/browser/extensions/api/app_window/app_window_api.h"
 
 #include "base/command_line.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/time/time.h"
@@ skipping 32 matching lines @@
     "The color specification could not be parsed.";
 const char kColorWithFrameNone[] = "Windows with no frame cannot have a color.";
 const char kInactiveColorWithoutColor[] =
     "frame.inactiveColor must be used with frame.color.";
 const char kConflictingBoundsOptions[] =
     "The $1 property cannot be specified for both inner and outer bounds.";
 const char kAlwaysOnTopPermission[] =
     "The \"app.window.alwaysOnTop\" permission is required.";
 const char kInvalidUrlParameter[] =
     "The URL used for window creation must be local for security reasons.";
+const char kImeWindowPermission[] =
+    "Only whitelisted component IME extensions can create ime window.";

[benwells, 2014/09/09 00:02:30]

Nit: s/window/windows/

[bshe, 2014/09/10 22:34:54]

Done.

 const char kAlphaEnabledWrongChannel[] =
     "The alphaEnabled option requires dev channel or newer.";
 const char kAlphaEnabledMissingPermission[] =
     "The alphaEnabled option requires app.window.alpha permission.";
 const char kAlphaEnabledNeedsFrameNone[] =
     "The alphaEnabled option can only be used with \"frame: 'none'\".";
 }  // namespace app_window_constants
 
 const char kNoneFrameOption[] = "none";
   // TODO(benwells): Remove HTML titlebar injection.
@@ skipping 130 matching lines @@
     if (!GetBoundsSpec(*options, &create_params, &error_))
       return false;
 
     if (!AppsClient::Get()->IsCurrentChannelOlderThanDev() ||
         extension()->location() == extensions::Manifest::COMPONENT) {
       if (options->type == extensions::api::app_window::WINDOW_TYPE_PANEL) {
         create_params.window_type = AppWindow::WINDOW_TYPE_PANEL;
       }
     }
 
+    if (options->ime.get() &&
+        extension()->location() == extensions::Manifest::COMPONENT) {
+      const char* whitelist[] = {
+        // Keep in sync with app.window's whitelist in _api_features.json
+        "06BE211D5F014BAB34BC22D9DDA09C63A81D828E",

[benwells, 2014/09/09 00:02:30]

I think the best way to do this checking is to create a new permission, similar
to the way alpha is done below.

That way you can use the _permission_features.json file to restrict the
permission to the one component extension, and also restrict it to dev channel.
(all development APIs should be resticted to dev channel until they are complete
and have gone through a final API review).

You don't need to do any whitelisting checking here (alphaEnabled does it but it
isn't really necessary).

[bshe, 2014/09/10 22:34:54]

Done.

+        "F94EE6AB36D6C6588670B2B01EB65212D9C64E33"
+      };
+      if (!extensions::SimpleFeature::IsIdInList(
+              extension_id(),
+              std::set<std::string>(whitelist,
+                                    whitelist + arraysize(whitelist)))) {
+        error_ = app_window_constants::kImeWindowPermission;
+        return false;
+      } else {
+        create_params.is_ime_window = true;

[benwells, 2014/09/09 00:02:30]

Should be setting to *options->ime.get() for cases where ime: false is
specified.

[bshe, 2014/09/10 22:34:54]

Oops. I thought options->ime.get() returns the boolean value.
Done.

+      }
+    }

[benwells, 2014/09/09 00:02:30]

There needs to be a check to make sure that if the client of this is not a
packaged app, create_params.is_ime_window is true. That needs to be enforced
somehow. Currently your component extension can create non-ime windows.

[bshe, 2014/09/10 22:34:54]

It is now enforced by whitelist. Only two whitelisted extension can request the
permission and hence create ime window.

It should be fine to allow whitelisted IMEs to create non-ime window IMO. Do you
have any specific concern?

On 2014/09/09 00:02:30, benwells wrote:

+
     if (!GetFrameOptions(*options, &create_params))
       return false;
 
     if (options->alpha_enabled.get()) {
       const char* whitelist[] = {
         "0F42756099D914A026DADFA182871C015735DD95",  // http://crbug.com/323773
         "2D22CDB6583FD0A13758AEBE8B15E45208B4E9A7",
         "E7E2461CE072DF036CF9592740196159E2D7C089",  // http://crbug.com/356200
         "A74A4D44C7CFCD8844830E6140C8D763E12DD8F3",
         "312745D9BF916161191143F6490085EEA0434997",
@@ skipping 275 matching lines @@
 
   if (options.frame->as_frame_options->inactive_color.get()) {
     error_ = app_window_constants::kInactiveColorWithoutColor;
     return false;
   }
 
   return true;
 }
 
 }  // namespace extensions