Migrate Declarative API bindings to new JSON objects generated by JSON compiler

chrome/browser/extensions/api/declarative/declarative_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/declarative/declarative_api.h"
 
+#include "base/bind.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/api/declarative/declarative_api_constants.h"
 #include "chrome/browser/extensions/api/declarative/rules_registry.h"
 #include "chrome/browser/extensions/api/declarative/rules_registry_service.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/common/extensions/api/experimental.declarative.h"
+#include "content/public/browser/browser_thread.h"
 
 namespace keys = extensions::declarative_api_constants;
 
+using namespace extensions::api::experimental_declarative;
+
 namespace {
 
 // Adds all entries from |list| to |out|. Assumes that all entries of |list|
 // are strings. Returns true if successful.
 bool AddAllStringValues(ListValue* list, std::vector<std::string>* out) {
   for (ListValue::iterator i = list->begin(); i != list->end(); ++i) {
     std::string value;
     if (!(*i)->GetAsString(&value))
       return false;
     out->push_back(value);
   }
   return true;
 }
 
 }  // namespace
 
 namespace extensions {
 
-bool AddRulesFunction::RunImpl() {
+RulesFunction::RulesFunction() : rules_registry_(NULL) {}
+
+bool RulesFunction::RunImpl() {
   std::string event_name;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &event_name));
-
-  ListValue* rules_list = NULL;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetList(1, &rules_list));
-
-  std::vector<DictionaryValue*> rules;
-  for (ListValue::iterator i = rules_list->begin();
-       i != rules_list->end();
-       ++i) {
-    DictionaryValue* rule = NULL;
-    EXTENSION_FUNCTION_VALIDATE((*i)->GetAsDictionary(&rule));
-    rules.push_back(rule);
-  }
+  EXTENSION_FUNCTION_VALIDATE(!args_->GetString(0, &event_name));

[not at google - send to devlin, 2012/02/18 02:59:01]

Shame to have to do this, I suppose you have no choice given the thread
complications.

I think the check was right before though as VALDATE(args) not VALIDATE(!args)?

[battre, 2012/02/20 10:17:36]

Obviously. 

Done.

 
   RulesRegistryService* rules_registry_service =
       profile()->GetExtensionService()->GetRulesRegistryService();
-  RulesRegistry* rules_registry =
-      rules_registry_service->GetRulesRegistry(event_name);
-  if (!rules_registry) {
+  rules_registry_ = rules_registry_service->GetRulesRegistry(event_name);
+  if (!rules_registry_) {
     error_ = keys::kInvalidEventName;
     return false;
   }

[not at google - send to devlin, 2012/02/18 02:59:01]

This (and other examples of the pattern) are the same as
EXTENSION_FUNCTION_VALIDATE(params.get())

[battre, 2012/02/20 10:17:36]

not quite. EXTENSION_FUNCTION_VALIDATE sets bad_message_ = true.

OTOH, if somebody sends us a request for a non-existing rules registry, that
should not happen, so let's change this.

Done.

[not at google - send to devlin, 2012/02/20 11:50:51]

Ah crap I think I put this comment on the wrong blob. I meant to put it on like
line 66/79/94 etc, such that

scoped_ptr<AddRules::Params> params(...);
if (!params.get()) {
  bad_message_ = true;
  return;
}

becomes

scoped_ptr<AddRules::Params> params(...);
EXTENSION_FUNCTION_VALIDATE(params.get());

and same for the other examples of that.

That said -- in this case you're right too, particularly now that the
declarative API is only available internally, there should never be a request
for a nonexistent rules registry. It might be nice to mention this in a comment.

[battre, 2012/02/28 22:29:18]

I wanted to do this change after landing http://codereview.chromium.org/9416060/
because PostTaskAndReply cannot rely the result of one function to another.
Ideally, this function would become some thing like this:

PostTaskAndReplyWithStatus(
    message_loop_proxy, FROM_HERE,
    base::Bind(&RulesFunction::RunImplOnCorrectThread, this),
    base::Bind(&RulesFunction::SendResponse, this),
    new bool);

I.e. we can get rid of SendResponseOnUIThread() entirely and pass the boolean
return value of RunImplOnCorrectThread to SendResponse.

Anyway, I don't see harm in letting RunImplOnCorrectThread return a boolean and
ignoring it for now.

Done.
Done.

 
-  error_ = rules_registry->AddRules(extension_id(), rules);
-  if (!error_.empty())
-    return false;
-
-  result_.reset(rules_list->DeepCopy());
-  return true;
-}
-
-bool RemoveRulesFunction::RunImpl() {
-  std::string event_name;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &event_name));
-
-  Value* rule_identifiers = NULL;
-  EXTENSION_FUNCTION_VALIDATE(args_->Get(1, &rule_identifiers));
-
-  RulesRegistryService* rules_registry_service =
-      profile()->GetExtensionService()->GetRulesRegistryService();
-  RulesRegistry* rules_registry =
-      rules_registry_service->GetRulesRegistry(event_name);
-  if (!rules_registry) {
-    error_ = keys::kInvalidEventName;
-    return false;
+  if (rules_registry_->GetOwnerThread() == content::BrowserThread::UI) {

[not at google - send to devlin, 2012/02/18 02:59:01]

I think this would be more clearly expressed as

content::BrowserThread::CurrentlyOn(rules_registry_->GetOwnerThread())

[battre, 2012/02/20 10:17:36]

Done.

+    RunImplOnCorrectThread();
+    SendResponseOnUIThread();
+  } else {
+    content::BrowserThread::PostTaskAndReply(
+        rules_registry_->GetOwnerThread(), FROM_HERE,
+        base::Bind(&RulesFunction::RunImplOnCorrectThread, this),
+        base::Bind(&RulesFunction::SendResponseOnUIThread, this));
   }
 
-  switch (rule_identifiers->GetType()) {
-    case Value::TYPE_NULL:
-      error_ = rules_registry->RemoveAllRules(extension_id());
-      break;
-    case Value::TYPE_LIST: {
-      std::vector<std::string> rule_identifiers_list;
-      EXTENSION_FUNCTION_VALIDATE(
-          AddAllStringValues(static_cast<ListValue*>(rule_identifiers),
-                             &rule_identifiers_list));
-      error_ = rules_registry->RemoveRules(extension_id(),
-                                           rule_identifiers_list);
-      break;
-    }
-    default:
-      error_ = keys::kInvalidDatatype;
-      break;
-  }
-  return error_.empty();
-}
-
-bool GetRulesFunction::RunImpl() {
-  std::string event_name;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetString(0, &event_name));
-
-  Value* rule_identifiers = NULL;
-  EXTENSION_FUNCTION_VALIDATE(args_->Get(1, &rule_identifiers));
-
-  RulesRegistryService* rules_registry_service =
-      profile()->GetExtensionService()->GetRulesRegistryService();
-  RulesRegistry* rules_registry =
-      rules_registry_service->GetRulesRegistry(event_name);
-  if (!rules_registry) {
-    error_ = keys::kInvalidEventName;
-    return false;
-  }
-
-  std::vector<DictionaryValue*> rules;
-  switch (rule_identifiers->GetType()) {
-    case Value::TYPE_NULL:
-      error_ = rules_registry->GetAllRules(extension_id(), &rules);
-      break;
-    case Value::TYPE_LIST: {
-      std::vector<std::string> rule_identifiers_list;
-      EXTENSION_FUNCTION_VALIDATE(
-          AddAllStringValues(static_cast<ListValue*>(rule_identifiers),
-                             &rule_identifiers_list));
-      error_ = rules_registry->GetRules(extension_id(), rule_identifiers_list,
-                                        &rules);
-      break;
-    }
-    default:
-      error_ = keys::kInvalidDatatype;
-      break;
-  }
-
-  if (!error_.empty())
-    return false;
-
-  scoped_ptr<ListValue> result(new ListValue);
-  for (std::vector<DictionaryValue*>::iterator i = rules.begin();
-      i != rules.end(); ++i)
-    result->Append(*i);
-  result_.reset(result.release());
-
   return true;
 }
 
+void RulesFunction::SendResponseOnUIThread() {
+  SendResponse(error_.empty() && !bad_message_);
+}
+
+void AddRulesFunction::RunImplOnCorrectThread() {
+  scoped_ptr<AddRules::Params> params(AddRules::Params::Create(*args_));
+  if (!params.get()) {
+    bad_message_ = true;
+    return;
+  }
+
+  error_ = rules_registry_->AddRules(extension_id(), params->rules);
+
+  if (error_.empty())
+    result_.reset(AddRules::Result::Create(params->rules));
+}
+
+void RemoveRulesFunction::RunImplOnCorrectThread() {
+  scoped_ptr<RemoveRules::Params> params(RemoveRules::Params::Create(*args_));
+  if (!params.get()) {
+    bad_message_ = true;
+    return;
+  }
+
+  if (params->rule_identifiers.get()) {
+    error_ = rules_registry_->RemoveRules(extension_id(),
+                                          *params->rule_identifiers);
+  } else {
+    error_ = rules_registry_->RemoveAllRules(extension_id());
+  }
+}
+
+void GetRulesFunction::RunImplOnCorrectThread() {
+  scoped_ptr<RemoveRules::Params> params(RemoveRules::Params::Create(*args_));
+  if (!params.get()) {
+    bad_message_ = true;
+    return;
+  }
+
+  std::vector<linked_ptr<Rule> > rules;
+  if (params->rule_identifiers.get()) {
+    error_ = rules_registry_->GetRules(extension_id(),
+                                       *params->rule_identifiers,
+                                       &rules);
+  } else {
+    error_ = rules_registry_->GetAllRules(extension_id(), &rules);
+  }
+
+  if (error_.empty())
+    result_.reset(GetRules::Result::Create(rules));
+}
+
 }  // namespace extensions