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

Index: chrome/browser/extensions/api/app_window/app_window_api.cc
diff --git a/chrome/browser/extensions/api/app_window/app_window_api.cc b/chrome/browser/extensions/api/app_window/app_window_api.cc
index 9b591a71f558bba21bcae2da008b0a17826940b4..e559ea8d0a3a188466497619d683147321844b2a 100644
--- a/chrome/browser/extensions/api/app_window/app_window_api.cc
+++ b/chrome/browser/extensions/api/app_window/app_window_api.cc
@@ -50,6 +50,8 @@ 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[] =
@@ -200,6 +202,24 @@ bool AppWindowCreateFunction::RunAsync() {
       }
     }
 
+    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;