Clean up windows API implementations to use JSON compiler generated code

chrome/browser/extensions/api/tabs/tabs_api.cc

Index: chrome/browser/extensions/api/tabs/tabs_api.cc
diff --git a/chrome/browser/extensions/api/tabs/tabs_api.cc b/chrome/browser/extensions/api/tabs/tabs_api.cc
index 09f86ab18a8f11ad1dd4dda2a0c2527b6e1ecd1b..91b3d3ac8956876f81c69db47289d21f3647ada1 100644
--- a/chrome/browser/extensions/api/tabs/tabs_api.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_api.cc
@@ -109,10 +109,15 @@ using content::WebContents;
 
 namespace extensions {
 
+namespace Create = api::windows::Create;
+namespace CreateData = Create::Params::CreateData;
 namespace Get = api::windows::Get;
 namespace GetAll = api::windows::GetAll;
 namespace GetCurrent = api::windows::GetCurrent;
 namespace GetLastFocused = api::windows::GetLastFocused;
+namespace Remove = api::windows::Remove;
+namespace Update = api::windows::Update;
+namespace UpdateInfo = Update::Params::UpdateInfo;

[not at google - send to devlin, 2013/08/21 15:04:36]

i really don't like all of these typedefs. Create, Get, etc are just way too
generic to be pulling up into the top level namespace like this.

Please just alias "namespace windows = api::windows".

[pals, 2013/08/22 08:54:28]

Done.

 namespace errors = extension_manifest_errors;
 namespace keys = tabs_constants;
 namespace tabs = api::tabs;
@@ -348,16 +353,15 @@ bool WindowsGetAllFunction::RunImpl() {
 }
 
 bool WindowsCreateFunction::ShouldOpenIncognitoWindow(
-    const base::DictionaryValue* args,
+    const Create::Params* args,
     std::vector<GURL>* urls,
     bool* is_error) {
   *is_error = false;
   const IncognitoModePrefs::Availability incognito_availability =
       IncognitoModePrefs::GetAvailability(profile_->GetPrefs());
   bool incognito = false;
-  if (args && args->HasKey(keys::kIncognitoKey)) {
-    EXTENSION_FUNCTION_VALIDATE(args->GetBoolean(keys::kIncognitoKey,
-                                                 &incognito));
+  if (args && args->create_data->incognito) {
+    incognito = *args->create_data->incognito;
     if (incognito && incognito_availability == IncognitoModePrefs::DISABLED) {
       error_ = keys::kIncognitoModeIsDisabled;
       *is_error = true;
@@ -398,36 +402,24 @@ bool WindowsCreateFunction::ShouldOpenIncognitoWindow(
 }
 
 bool WindowsCreateFunction::RunImpl() {
-  base::DictionaryValue* args = NULL;
+  scoped_ptr<Create::Params> args(Create::Params::Create(*args_));

[not at google - send to devlin, 2013/08/21 15:04:36]

args can't be null. the schema compilation should be generating the correct code
that handles whatever the schema can throw at it.

the thing to null-check against is probably args->create_data not args. So:

scoped_ptr<Create::Params> params(Create::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params);

// use this everywhere not |args|
Create::Params::CreateData* create_data = params->create_data.get();

[pals, 2013/08/22 08:54:28]

Done.

   std::vector<GURL> urls;
   TabStripModel* source_tab_strip = NULL;
   int tab_index = -1;
 
   if (HasOptionalArgument(0))
-    EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(0, &args));
+    EXTENSION_FUNCTION_VALIDATE(args);
 
   // Look for optional url.
   if (args) {
-    if (args->HasKey(keys::kUrlKey)) {
-      Value* url_value;
+    if (args->create_data->url) {
       std::vector<std::string> url_strings;
-      args->Get(keys::kUrlKey, &url_value);
-
       // First, get all the URLs the client wants to open.
-      if (url_value->IsType(Value::TYPE_STRING)) {
-        std::string url_string;
-        url_value->GetAsString(&url_string);
-        url_strings.push_back(url_string);
-      } else if (url_value->IsType(Value::TYPE_LIST)) {
-        const base::ListValue* url_list =
-            static_cast<const base::ListValue*>(url_value);
-        for (size_t i = 0; i < url_list->GetSize(); ++i) {
-          std::string url_string;
-          EXTENSION_FUNCTION_VALIDATE(url_list->GetString(i, &url_string));
-          url_strings.push_back(url_string);
-        }
+      if (args->create_data->url->as_string) {
+        url_strings.push_back(*args->create_data->url->as_string);
+      } else if (args->create_data->url->as_strings) {
+        url_strings = *args->create_data->url->as_strings;
       }
-
       // Second, resolve, validate and convert them to GURLs.
       for (std::vector<std::string>::iterator i = url_strings.begin();
            i != url_strings.end(); ++i) {
@@ -450,13 +442,11 @@ bool WindowsCreateFunction::RunImpl() {
 
   // Look for optional tab id.
   if (args) {
-    int tab_id = -1;
-    if (args->HasKey(keys::kTabIdKey)) {
-      EXTENSION_FUNCTION_VALIDATE(args->GetInteger(keys::kTabIdKey, &tab_id));
-
+    if (args->create_data->tab_id) {
       // Find the tab. |source_tab_strip| and |tab_index| will later be used to
       // move the tab into the created window.
-      if (!GetTabById(tab_id, profile(), include_incognito(),
+      if (!GetTabById(*args->create_data->tab_id, profile(),
+                      include_incognito(),
                       NULL, &source_tab_strip,
                       NULL, &tab_index, &error_))
         return false;
@@ -477,7 +467,7 @@ bool WindowsCreateFunction::RunImpl() {
 
   // Decide whether we are opening a normal window or an incognito window.
   bool is_error = true;
-  bool open_incognito_window = ShouldOpenIncognitoWindow(args, &urls,
+  bool open_incognito_window = ShouldOpenIncognitoWindow(args.get(), &urls,
                                                          &is_error);
   if (is_error) {
     // error_ member variable is set inside of ShouldOpenIncognitoWindow.
@@ -490,15 +480,12 @@ bool WindowsCreateFunction::RunImpl() {
   if (args) {
     // Figure out window type before figuring out bounds so that default
     // bounds can be set according to the window type.
-    std::string type_str;
-    if (args->HasKey(keys::kWindowTypeKey)) {
-      EXTENSION_FUNCTION_VALIDATE(args->GetString(keys::kWindowTypeKey,
-                                                  &type_str));
-      if (type_str == keys::kWindowTypeValuePopup) {
+    if (args->create_data->type != CreateData::TYPE_NONE) {

[not at google - send to devlin, 2013/08/21 15:04:36]

could you restructure this to use switch()?

[pals, 2013/08/22 08:54:28]

Done.

+      if (args->create_data->type == CreateData::TYPE_POPUP) {
         window_type = Browser::TYPE_POPUP;
         extension_id = GetExtension()->id();
-      } else if (type_str == keys::kWindowTypeValuePanel ||
-                 type_str == keys::kWindowTypeValueDetachedPanel) {
+      } else if (args->create_data->type == CreateData::TYPE_PANEL ||
+                 args->create_data->type == CreateData::TYPE_DETACHED_PANEL) {
         extension_id = GetExtension()->id();
         bool use_panels = false;
 #if !defined(OS_ANDROID)
@@ -508,13 +495,13 @@ bool WindowsCreateFunction::RunImpl() {
           create_panel = true;
 #if !defined(OS_CHROMEOS)
           // Non-ChromeOS has both docked and detached panel types.
-          if (type_str == keys::kWindowTypeValueDetachedPanel)
+          if (args->create_data->type == CreateData::TYPE_DETACHED_PANEL)
             panel_create_mode = PanelManager::CREATE_AS_DETACHED;
 #endif
         } else {
           window_type = Browser::TYPE_POPUP;
         }
-      } else if (type_str != keys::kWindowTypeValueNormal) {
+      } else if (args->create_data->type != CreateData::TYPE_NORMAL) {
         error_ = keys::kInvalidWindowTypeError;
         return false;
       }
@@ -545,36 +532,20 @@ bool WindowsCreateFunction::RunImpl() {
     }
 
     // Any part of the bounds can optionally be set by the caller.
-    int bounds_val = -1;
-    if (args->HasKey(keys::kLeftKey)) {
-      EXTENSION_FUNCTION_VALIDATE(args->GetInteger(keys::kLeftKey,
-                                                   &bounds_val));
-      window_bounds.set_x(bounds_val);
-    }
+    if (args->create_data->left)
+      window_bounds.set_x(*args->create_data->left);
 
-    if (args->HasKey(keys::kTopKey)) {
-      EXTENSION_FUNCTION_VALIDATE(args->GetInteger(keys::kTopKey,
-                                                   &bounds_val));
-      window_bounds.set_y(bounds_val);
-    }
+    if (args->create_data->top)
+      window_bounds.set_y(*args->create_data->top);
 
-    if (args->HasKey(keys::kWidthKey)) {
-      EXTENSION_FUNCTION_VALIDATE(args->GetInteger(keys::kWidthKey,
-                                                   &bounds_val));
-      window_bounds.set_width(bounds_val);
-    }
+    if (args->create_data->width)
+      window_bounds.set_width(*args->create_data->width);
 
-    if (args->HasKey(keys::kHeightKey)) {
-      EXTENSION_FUNCTION_VALIDATE(args->GetInteger(keys::kHeightKey,
-                                                   &bounds_val));
-      window_bounds.set_height(bounds_val);
-    }
+    if (args->create_data->height)
+      window_bounds.set_height(*args->create_data->height);
 
-    if (args->HasKey(keys::kFocusedKey)) {
-      EXTENSION_FUNCTION_VALIDATE(args->GetBoolean(keys::kFocusedKey,
-                                                   &focused));
-      saw_focus_key = true;
-    }
+    if (args->create_data->focused)
+      saw_focus_key = *args->create_data->focused;
   }
 
   if (create_panel) {
@@ -688,13 +659,11 @@ bool WindowsCreateFunction::RunImpl() {
 }
 
 bool WindowsUpdateFunction::RunImpl() {
-  int window_id = extension_misc::kUnknownWindowId;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &window_id));
-  base::DictionaryValue* update_props;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetDictionary(1, &update_props));
+  scoped_ptr<Update::Params> params(Update::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params);
 
   WindowController* controller;
-  if (!GetWindowFromWindowID(this, window_id, &controller))
+  if (!GetWindowFromWindowID(this, params->window_id, &controller))
     return false;
 
 #if defined(OS_WIN)
@@ -706,17 +675,14 @@ bool WindowsUpdateFunction::RunImpl() {
 #endif
 
   ui::WindowShowState show_state = ui::SHOW_STATE_DEFAULT;  // No change.
-  std::string state_str;
-  if (update_props->HasKey(keys::kShowStateKey)) {
-    EXTENSION_FUNCTION_VALIDATE(update_props->GetString(keys::kShowStateKey,
-                                                        &state_str));
-    if (state_str == keys::kShowStateValueNormal) {
+  if (params->update_info.state != UpdateInfo::STATE_NONE) {
+    if (params->update_info.state == UpdateInfo::STATE_NORMAL) {
       show_state = ui::SHOW_STATE_NORMAL;
-    } else if (state_str == keys::kShowStateValueMinimized) {
+    } else if (params->update_info.state == UpdateInfo::STATE_MINIMIZED) {
       show_state = ui::SHOW_STATE_MINIMIZED;
-    } else if (state_str == keys::kShowStateValueMaximized) {
+    } else if (params->update_info.state == UpdateInfo::STATE_MAXIMIZED) {
       show_state = ui::SHOW_STATE_MAXIMIZED;
-    } else if (state_str == keys::kShowStateValueFullscreen) {
+    } else if (params->update_info.state == UpdateInfo::STATE_FULLSCREEN) {
       show_state = ui::SHOW_STATE_FULLSCREEN;
     } else {
       error_ = keys::kInvalidWindowStateError;
@@ -756,36 +722,23 @@ bool WindowsUpdateFunction::RunImpl() {
   bool set_bounds = false;
 
   // Any part of the bounds can optionally be set by the caller.
-  int bounds_val;
-  if (update_props->HasKey(keys::kLeftKey)) {
-    EXTENSION_FUNCTION_VALIDATE(update_props->GetInteger(
-        keys::kLeftKey,
-        &bounds_val));
-    bounds.set_x(bounds_val);
+  if (params->update_info.left) {
+    bounds.set_x(*params->update_info.left);
     set_bounds = true;
   }
 
-  if (update_props->HasKey(keys::kTopKey)) {
-    EXTENSION_FUNCTION_VALIDATE(update_props->GetInteger(
-        keys::kTopKey,
-        &bounds_val));
-    bounds.set_y(bounds_val);
+  if (params->update_info.top) {
+    bounds.set_y(*params->update_info.top);
     set_bounds = true;
   }
 
-  if (update_props->HasKey(keys::kWidthKey)) {
-    EXTENSION_FUNCTION_VALIDATE(update_props->GetInteger(
-        keys::kWidthKey,
-        &bounds_val));
-    bounds.set_width(bounds_val);
+  if (params->update_info.width) {
+    bounds.set_width(*params->update_info.width);
     set_bounds = true;
   }
 
-  if (update_props->HasKey(keys::kHeightKey)) {
-    EXTENSION_FUNCTION_VALIDATE(update_props->GetInteger(
-        keys::kHeightKey,
-        &bounds_val));
-    bounds.set_height(bounds_val);
+  if (params->update_info.height) {
+    bounds.set_height(*params->update_info.height);
     set_bounds = true;
   }
 
@@ -801,11 +754,8 @@ bool WindowsUpdateFunction::RunImpl() {
     controller->window()->SetBounds(bounds);
   }
 
-  bool active_val = false;
-  if (update_props->HasKey(keys::kFocusedKey)) {
-    EXTENSION_FUNCTION_VALIDATE(update_props->GetBoolean(
-        keys::kFocusedKey, &active_val));
-    if (active_val) {
+  if (params->update_info.focused) {
+    if (*params->update_info.focused) {
       if (show_state == ui::SHOW_STATE_MINIMIZED) {
         error_ = keys::kInvalidWindowStateError;
         return false;
@@ -821,12 +771,8 @@ bool WindowsUpdateFunction::RunImpl() {
     }
   }
 
-  bool draw_attention = false;
-  if (update_props->HasKey(keys::kDrawAttentionKey)) {
-    EXTENSION_FUNCTION_VALIDATE(update_props->GetBoolean(
-        keys::kDrawAttentionKey, &draw_attention));
-    controller->window()->FlashFrame(draw_attention);
-  }
+  if (params->update_info.draw_attention)
+    controller->window()->FlashFrame(*params->update_info.draw_attention);
 
   SetResult(controller->CreateWindowValue());
 
@@ -834,11 +780,11 @@ bool WindowsUpdateFunction::RunImpl() {
 }
 
 bool WindowsRemoveFunction::RunImpl() {
-  int window_id = -1;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &window_id));
+  scoped_ptr<Remove::Params> params(Remove::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params);
 
   WindowController* controller;
-  if (!GetWindowFromWindowID(this, window_id, &controller))
+  if (!GetWindowFromWindowID(this, params->window_id, &controller))
     return false;
 
 #if defined(OS_WIN)