Remove service worker concepts from _api_features.json

Remove service worker concepts from _api_features.json

BUG=396340
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285819

[chromium-reviews, 2014-07-25 14:43:06]

Ping
On Jul 22, 2014 4:52 PM, <scheib@chromium.org> wrote:

> Reviewers: Jeffrey Yasskin,
>
> Description:
> Remove service worker concepts from _api_features.json
>
> BUG=396340
>
> Please review this at https://codereview.chromium.org/414633002/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files (+2, -82 lines):
>   M extensions/common/features/api_feature.h
>   M extensions/common/features/api_feature.cc
>   M extensions/common/features/complex_feature.h
>   M extensions/common/features/complex_feature.cc
>   M extensions/common/features/complex_feature_unittest.cc
>   M extensions/common/features/feature.h
>   M extensions/common/features/simple_feature.h
>   M extensions/common/features/simple_feature.cc
>
>
> Index: extensions/common/features/api_feature.cc
> diff --git a/extensions/common/features/api_feature.cc
> b/extensions/common/features/api_feature.cc
> index fc7f707be4f50046043ea8ec90044a074ba6e1df..
> e3e4c91b6db3796a5d548d38b6ef5bd9576fca3e 100644
> --- a/extensions/common/features/api_feature.cc
> +++ b/extensions/common/features/api_feature.cc
> @@ -7,7 +7,7 @@
>  namespace extensions {
>
>  APIFeature::APIFeature()
> -    : internal_(false), blocked_in_service_worker_(false) {}
> +    : internal_(false) {}
>
>  APIFeature::~APIFeature() {
>  }
> @@ -16,17 +16,12 @@ bool APIFeature::IsInternal() const {
>    return internal_;
>  }
>
> -bool APIFeature::IsBlockedInServiceWorker() const {
> -  return blocked_in_service_worker_;
> -}
> -
>  std::string APIFeature::Parse(const base::DictionaryValue* value) {
>    std::string error = SimpleFeature::Parse(value);
>    if (!error.empty())
>      return error;
>
>    value->GetBoolean("internal", &internal_);
> -  value->GetBoolean("blocked_in_service_worker",
> &blocked_in_service_worker_);
>
>    if (contexts()->empty())
>      return name() + ": API features must specify at least one context.";
> Index: extensions/common/features/api_feature.h
> diff --git a/extensions/common/features/api_feature.h
> b/extensions/common/features/api_feature.h
> index 1e57b8dc322f8d47c0f7f0fc40a7c45465d16a74..
> c146f9d914b0dce0cfcae3ce6d04ea7ac07d44d6 100644
> --- a/extensions/common/features/api_feature.h
> +++ b/extensions/common/features/api_feature.h
> @@ -18,13 +18,11 @@ class APIFeature : public SimpleFeature {
>
>    // extensions::Feature:
>    virtual bool IsInternal() const OVERRIDE;
> -  virtual bool IsBlockedInServiceWorker() const OVERRIDE;
>
>    virtual std::string Parse(const base::DictionaryValue* value) OVERRIDE;
>
>   private:
>    bool internal_;
> -  bool blocked_in_service_worker_;
>  };
>
>  }  // namespace extensions
> Index: extensions/common/features/complex_feature.cc
> diff --git a/extensions/common/features/complex_feature.cc
> b/extensions/common/features/complex_feature.cc
> index 94402859e7120f4a9ae593d5c934613e005fcfaa..
> 674b4eea741c4c3dd283f2dfe4e586c34303c9ec 100644
> --- a/extensions/common/features/complex_feature.cc
> +++ b/extensions/common/features/complex_feature.cc
> @@ -12,20 +12,14 @@ ComplexFeature::ComplexFeature(scoped_ptr<FeatureList>
> features) {
>    no_parent_ = features_[0]->no_parent();
>
>  #if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
> -  // Verify IsInternal and IsBlockedInServiceWorker are consistent across
> all
> -  // features.
> +  // Verify IsInternal and no_parent is consistent across all features.
>    bool first_is_internal = features_[0]->IsInternal();
> -  bool first_blocked_in_service_worker =
> -      features_[0]->IsBlockedInServiceWorker();
>    for (FeatureList::const_iterator it = features_.begin() + 1;
>         it != features_.end();
>         ++it) {
>      DCHECK(first_is_internal == (*it)->IsInternal())
>          << "Complex feature must have consistent values of "
>             "internal across all sub features.";
> -    DCHECK(first_blocked_in_service_worker == (*it)->
> IsBlockedInServiceWorker())
> -        << "Complex feature must have consistent values of "
> -           "blocked_in_service_worker across all sub features.";
>      DCHECK(no_parent_ == (*it)->no_parent())
>          << "Complex feature must have consistent values of "
>             "no_parent across all sub features.";
> @@ -102,12 +96,6 @@ bool ComplexFeature::IsIdInWhitelist(const
> std::string& extension_id) const {
>    return false;
>  }
>
> -bool ComplexFeature::IsBlockedInServiceWorker() const {
> -  // Constructor verifies that composed features are consistent, thus we
> can
> -  // return just the first feature's value.
> -  return features_[0]->IsBlockedInServiceWorker();
> -}
> -
>  bool ComplexFeature::IsInternal() const {
>    // Constructor verifies that composed features are consistent, thus we
> can
>    // return just the first feature's value.
> Index: extensions/common/features/complex_feature.h
> diff --git a/extensions/common/features/complex_feature.h
> b/extensions/common/features/complex_feature.h
> index 9204b256cd46b5d6fd84dde1b91e06339c002ade..
> bbb73e483d3a6cbf3eb53347f4038a4da846b34d 100644
> --- a/extensions/common/features/complex_feature.h
> +++ b/extensions/common/features/complex_feature.h
> @@ -39,7 +39,6 @@ class ComplexFeature : public Feature {
>
>    virtual bool IsIdInBlacklist(const std::string& extension_id) const
> OVERRIDE;
>    virtual bool IsIdInWhitelist(const std::string& extension_id) const
> OVERRIDE;
> -  virtual bool IsBlockedInServiceWorker() const OVERRIDE;
>
>   protected:
>    // extensions::Feature:
> Index: extensions/common/features/complex_feature_unittest.cc
> diff --git a/extensions/common/features/complex_feature_unittest.cc
> b/extensions/common/features/complex_feature_unittest.cc
> index bef056d92fa319bf5d0723403cd7ae24a4450730..
> 54e5286342d101e42a71dba0528d1ab1304d5122 100644
> --- a/extensions/common/features/complex_feature_unittest.cc
> +++ b/extensions/common/features/complex_feature_unittest.cc
> @@ -6,14 +6,11 @@
>
>  #include "chrome/common/extensions/features/chrome_channel_
> feature_filter.h"
>  #include "chrome/common/extensions/features/feature_channel.h"
> -#include "extensions/common/features/api_feature.h"
>  #include "extensions/common/features/simple_feature.h"
> -#include "extensions/common/test_util.h"
>  #include "extensions/common/value_builder.h"
>  #include "testing/gtest/include/gtest/gtest.h"
>
>  using chrome::VersionInfo;
> -using extensions::APIFeature;
>  using extensions::ComplexFeature;
>  using extensions::DictionaryBuilder;
>  using extensions::Feature;
> @@ -21,7 +18,6 @@ using extensions::ListBuilder;
>  using extensions::Manifest;
>  using extensions::ScopedCurrentChannel;
>  using extensions::SimpleFeature;
> -using extensions::test_util::ParseJsonDictionaryWithSingleQuotes;
>
>  namespace {
>
> @@ -165,56 +161,6 @@ TEST_F(ExtensionComplexFeatureTest,
> MultipleRulesChannels) {
>    }
>  }
>
> -// Tests a complex feature with blocked_in_service_worker returns true for
> -// IsBlockedInServiceWorker().
> -TEST_F(ExtensionComplexFeatureTest, BlockedInServiceWorker) {
> -  scoped_ptr<ComplexFeature::FeatureList> features(
> -      new ComplexFeature::FeatureList());
> -
> -  // Two feature rules, both with blocked_in_service_worker: true.
> -  scoped_ptr<SimpleFeature> api_feature(new APIFeature());
> -  api_feature->Parse(ParseJsonDictionaryWithSingleQuotes(
> -                         "{"
> -                         "  'channel': 'trunk',"
> -                         "  'blocked_in_service_worker': true"
> -                         "}").get());
> -  features->push_back(api_feature.release());
> -
> -  api_feature.reset(new APIFeature());
> -  api_feature->Parse(ParseJsonDictionaryWithSingleQuotes(
> -                         "{"
> -                         "  'channel': 'stable',"
> -                         "  'blocked_in_service_worker': true"
> -                         "}").get());
> -  features->push_back(api_feature.release());
> -
> -  EXPECT_TRUE(ComplexFeature(features.Pass()).
> IsBlockedInServiceWorker());
> -}
> -
> -// Tests a complex feature without blocked_in_service_worker returns
> false for
> -// IsBlockedInServiceWorker().
> -TEST_F(ExtensionComplexFeatureTest, NotBlockedInServiceWorker) {
> -  scoped_ptr<ComplexFeature::FeatureList> features(
> -      new ComplexFeature::FeatureList());
> -
> -  // Two feature rules without blocked_in_service_worker.
> -  scoped_ptr<SimpleFeature> api_feature(new APIFeature());
> -  api_feature->Parse(ParseJsonDictionaryWithSingleQuotes(
> -                         "{"
> -                         "  'channel': 'trunk'"
> -                         "}").get());
> -  features->push_back(api_feature.release());
> -
> -  api_feature.reset(new APIFeature());
> -  api_feature->Parse(ParseJsonDictionaryWithSingleQuotes(
> -                         "{"
> -                         "  'channel': 'stable'"
> -                         "}").get());
> -  features->push_back(api_feature.release());
> -
> -  EXPECT_FALSE(ComplexFeature(features.Pass()).
> IsBlockedInServiceWorker());
> -}
> -
>  // Tests that dependencies are correctly checked.
>  TEST_F(ExtensionComplexFeatureTest, Dependencies) {
>    scoped_ptr<ComplexFeature::FeatureList> features(
> Index: extensions/common/features/feature.h
> diff --git a/extensions/common/features/feature.h
> b/extensions/common/features/feature.h
> index 37387d7491959be4001f133ee69e882d4ec32f5f..
> 97b18147f183f1e69b82e17b407164ad6aa32077 100644
> --- a/extensions/common/features/feature.h
> +++ b/extensions/common/features/feature.h
> @@ -111,9 +111,6 @@ class Feature {
>    // Tests whether this is an internal API or not.
>    virtual bool IsInternal() const = 0;
>
> -  // Returns True for features excluded from service worker backed
> contexts.
> -  virtual bool IsBlockedInServiceWorker() const = 0;
> -
>    // Returns true if the feature is available to be parsed into a new
> extension
>    // manifest.
>    Availability IsAvailableToManifest(const std::string& extension_id,
> Index: extensions/common/features/simple_feature.cc
> diff --git a/extensions/common/features/simple_feature.cc
> b/extensions/common/features/simple_feature.cc
> index 155e76429be855f76dcd833b16a9799c47239c7c..
> d66d1e83f0f83db118759e29e126efc3e47d13e7 100644
> --- a/extensions/common/features/simple_feature.cc
> +++ b/extensions/common/features/simple_feature.cc
> @@ -510,8 +510,6 @@ bool SimpleFeature::IsInternal() const {
>    return false;
>  }
>
> -bool SimpleFeature::IsBlockedInServiceWorker() const { return false; }
> -
>  bool SimpleFeature::IsIdInBlacklist(const std::string& extension_id)
> const {
>    return IsIdInList(extension_id, blacklist_);
>  }
> Index: extensions/common/features/simple_feature.h
> diff --git a/extensions/common/features/simple_feature.h
> b/extensions/common/features/simple_feature.h
> index 7df6e19b0b779cb1f2974d81ec185d24b4fb935a..
> 395e7e70c9fc2cf309f2c806fc59eb41eff28693 100644
> --- a/extensions/common/features/simple_feature.h
> +++ b/extensions/common/features/simple_feature.h
> @@ -106,7 +106,6 @@ class SimpleFeature : public Feature {
>                                               Context context) const
> OVERRIDE;
>
>    virtual bool IsInternal() const OVERRIDE;
> -  virtual bool IsBlockedInServiceWorker() const OVERRIDE;
>
>    virtual bool IsIdInBlacklist(const std::string& extension_id) const
> OVERRIDE;
>    virtual bool IsIdInWhitelist(const std::string& extension_id) const
> OVERRIDE;
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Jeffrey Yasskin, 2014-07-25 23:17:40]

lgtm

[scheib, 2014-07-25 23:41:52]

The CQ bit was checked by scheib@chromium.org

[commit-bot: I haz the power, 2014-07-25 23:45:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/scheib@chromium.org/414633002/1

[commit-bot: I haz the power, 2014-07-26 18:09:58]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-07-26 18:19:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-26 18:19:05]

Try jobs failed on following builders:
  linux_chromium_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[scheib, 2014-07-26 23:35:37]

The CQ bit was checked by scheib@chromium.org

[commit-bot: I haz the power, 2014-07-26 23:36:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/scheib@chromium.org/414633002/1

[commit-bot: I haz the power, 2014-07-26 23:56:03]

Change committed as 285819