Clean up windows API implementations to use JSON compiler generated code

chrome/browser/extensions/api/tabs/tabs_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/tabs/tabs_api.h"
 
 #include <algorithm>
 #include <limits>
 #include <vector>
 
@@ skipping 91 matching lines @@
 using content::BrowserThread;
 using content::NavigationController;
 using content::NavigationEntry;
 using content::OpenURLParams;
 using content::Referrer;
 using content::RenderViewHost;
 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;
 typedef tabs::CaptureVisibleTab::Params::Options FormatEnum;
 
 using api::tabs::InjectDetails;
 
 const int TabsCaptureVisibleTabFunction::kDefaultQuality = 90;
 
 namespace {
@@ skipping 215 matching lines @@
     if (populate_tabs)
       window_list->Append((*iter)->CreateWindowValueWithTabs(GetExtension()));
     else
       window_list->Append((*iter)->CreateWindowValue());
   }
   SetResult(window_list);
   return true;
 }
 
 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;
       return false;
     }
     if (!incognito && incognito_availability == IncognitoModePrefs::FORCED) {
       error_ = keys::kIncognitoModeIsForced;
       *is_error = true;
       return false;
     }
@@ skipping 20 matching lines @@
       error_ = ErrorUtils::FormatErrorMessage(
           keys::kURLsNotAllowedInIncognitoError, first_url_erased);
       *is_error = true;
       return false;
     }
   }
   return incognito;
 }
 
 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) {
         GURL url = ExtensionTabUtil::ResolvePossiblyRelativeURL(
             *i, GetExtension());
         if (!url.is_valid()) {
           error_ = ErrorUtils::FormatErrorMessage(
               keys::kInvalidUrlError, *i);
           return false;
         }
         // Don't let the extension crash the browser or renderers.
         if (ExtensionTabUtil::IsCrashURL(url)) {
           error_ = keys::kNoCrashBrowserError;
           return false;
         }
         urls.push_back(url);
       }
     }
   }
 
   // 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;
     }
   }
 
   Profile* window_profile = profile();
   Browser::Type window_type = Browser::TYPE_TABBED;
   bool create_panel = false;
 
   // panel_create_mode only applies if create_panel = true
   PanelManager::CreateMode panel_create_mode = PanelManager::CREATE_AS_DOCKED;
 
   gfx::Rect window_bounds;
   bool focused = true;
   bool saw_focus_key = false;
   std::string extension_id;
 
   // 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.
     return false;
   }
   if (open_incognito_window) {
     window_profile = window_profile->GetOffTheRecordProfile();
   }
 
   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)
         use_panels = PanelManager::ShouldUsePanels(extension_id);
 #endif
         if (use_panels) {
           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;
       }
     }
 
     // Initialize default window bounds according to window type.
     if (window_type == Browser::TYPE_TABBED ||
         window_type == Browser::TYPE_POPUP ||
         create_panel) {
       // Try to position the new browser relative to its originating
@@ skipping 10 matching lines @@
                                                       &window_bounds,
                                                       &show_state);
     }
 
     if (create_panel && PanelManager::CREATE_AS_DETACHED == panel_create_mode) {
       window_bounds.set_origin(
           PanelManager::GetInstance()->GetDefaultDetachedPanelOrigin());
     }
 
     // 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) {
     if (urls.empty())
       urls.push_back(GURL(chrome::kChromeUINewTabURL));
 
 #if defined(OS_CHROMEOS)
     if (PanelManager::ShouldUsePanels(extension_id)) {
       ShellWindow::CreateParams create_params;
       create_params.window_type = ShellWindow::WINDOW_TYPE_V1_PANEL;
@@ skipping 93 matching lines @@
   } else {
     SetResult(
         new_window->extension_window_controller()->CreateWindowValueWithTabs(
             GetExtension()));
   }
 
   return true;
 }
 
 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)
   // Silently ignore changes on the window for metro mode.
   if (win8::IsSingleWindowMetroMode()) {
     SetResult(controller->CreateWindowValue());
     return true;
   }
 #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;
       return false;
     }
   }
 
   if (show_state != ui::SHOW_STATE_FULLSCREEN &&
       show_state != ui::SHOW_STATE_DEFAULT)
     controller->SetFullscreenMode(false, GetExtension()->url());
@@ skipping 19 matching lines @@
   }
 
   gfx::Rect bounds;
   if (controller->window()->IsMinimized())
     bounds = controller->window()->GetRestoredBounds();
   else
     bounds = controller->window()->GetBounds();
   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;
   }
 
   if (set_bounds) {
     if (show_state == ui::SHOW_STATE_MINIMIZED ||
         show_state == ui::SHOW_STATE_MAXIMIZED ||
         show_state == ui::SHOW_STATE_FULLSCREEN) {
       error_ = keys::kInvalidWindowStateError;
       return false;
     }
     // TODO(varkha): Updating bounds during a drag can cause problems and a more
     // general solution is needed. See http://crbug.com/251813 .
     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;
       }
       controller->window()->Activate();
     } else {
       if (show_state == ui::SHOW_STATE_MAXIMIZED ||
           show_state == ui::SHOW_STATE_FULLSCREEN) {
         error_ = keys::kInvalidWindowStateError;
         return false;
       }
       controller->window()->Deactivate();
     }
   }
 
-  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());
 
   return true;
 }
 
 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)
   // In Windows 8 metro mode, an existing Browser instance is reused for
   // hosting the extension tab. We should not be closing it as we don't own it.
   if (win8::IsSingleWindowMetroMode())
     return false;
 #endif
 
   WindowController::Reason reason;
@@ skipping 1213 matching lines @@
   execute_tab_id_ = tab_id;
   details_ = details.Pass();
   return true;
 }
 
 bool TabsInsertCSSFunction::ShouldInsertCSS() const {
   return true;
 }
 
 }  // namespace extensions