diff options
| -rw-r--r-- | chrome/browser/chromeos/plugin_selection_policy.cc | 13 | ||||
| -rw-r--r-- | chrome/browser/chromeos/plugin_selection_policy.h | 12 | ||||
| -rw-r--r-- | chrome/browser/chromeos/plugin_selection_policy_unittest.cc | 122 |
3 files changed, 87 insertions, 60 deletions
diff --git a/chrome/browser/chromeos/plugin_selection_policy.cc b/chrome/browser/chromeos/plugin_selection_policy.cc index b27bc42..477965a 100644 --- a/chrome/browser/chromeos/plugin_selection_policy.cc +++ b/chrome/browser/chromeos/plugin_selection_policy.cc @@ -33,7 +33,8 @@ namespace chromeos { static const char kPluginSelectionPolicyFile[] = "/usr/share/chromeos-assets/flash/plugin_policy"; -PluginSelectionPolicy::PluginSelectionPolicy() : initialized_(false) { +PluginSelectionPolicy::PluginSelectionPolicy() + : init_from_file_finished_(false) { } void PluginSelectionPolicy::StartInit() { @@ -52,15 +53,13 @@ bool PluginSelectionPolicy::InitFromFile(const FilePath& policy_file) { // This must always be called from the FILE thread. DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - // Note: We're "initialized" even if loading the file fails. - initialized_ = true; - string data; // This should be a really small file, so we're OK with just // slurping it. if (!file_util::ReadFileToString(policy_file, &data)) { LOG(ERROR) << "Unable to read plugin policy file \"" << policy_file.value() << "\"."; + init_from_file_finished_ = true; return false; } @@ -81,6 +80,7 @@ bool PluginSelectionPolicy::InitFromFile(const FilePath& policy_file) { // Has to be preceeded by a "plugin" statement. if (last_plugin.empty()) { LOG(ERROR) << "Plugin policy file error: 'allow' out of context."; + init_from_file_finished_ = true; return false; } line = line.substr(6); @@ -92,6 +92,7 @@ bool PluginSelectionPolicy::InitFromFile(const FilePath& policy_file) { // Has to be preceeded by a "plugin" statement. if (last_plugin.empty()) { LOG(ERROR) << "Plugin policy file error: 'deny' out of context."; + init_from_file_finished_ = true; return false; } line = line.substr(5); @@ -113,6 +114,7 @@ bool PluginSelectionPolicy::InitFromFile(const FilePath& policy_file) { policies.insert(make_pair(last_plugin, policy)); policies_.swap(policies); + init_from_file_finished_ = true; return true; } @@ -136,7 +138,8 @@ bool PluginSelectionPolicy::IsAllowed(const GURL& url, // initialization is complete. Right now it is guaranteed only by // the startup order and the fact that InitFromFile runs on the FILE // thread too. - DCHECK(initialized_) << "Tried to check policy before policy is initialized."; + DCHECK(init_from_file_finished_) + << "Tried to check policy before policy is initialized."; string name = path.BaseName().value(); diff --git a/chrome/browser/chromeos/plugin_selection_policy.h b/chrome/browser/chromeos/plugin_selection_policy.h index 4786e3c..1bf60a0 100644 --- a/chrome/browser/chromeos/plugin_selection_policy.h +++ b/chrome/browser/chromeos/plugin_selection_policy.h @@ -56,9 +56,10 @@ class PluginSelectionPolicy private: // To allow access to InitFromFile FRIEND_TEST_ALL_PREFIXES(PluginSelectionPolicyTest, Basic); + FRIEND_TEST_ALL_PREFIXES(PluginSelectionPolicyTest, FindFirstAllowed); FRIEND_TEST_ALL_PREFIXES(PluginSelectionPolicyTest, InitFromFile); FRIEND_TEST_ALL_PREFIXES(PluginSelectionPolicyTest, IsAllowed); - FRIEND_TEST_ALL_PREFIXES(PluginSelectionPolicyTest, FindFirstAllowed); + FRIEND_TEST_ALL_PREFIXES(PluginSelectionPolicyTest, MissingFile); // Initializes from the default policy file. bool Init(); @@ -70,7 +71,14 @@ class PluginSelectionPolicy typedef std::map<std::string, Policy> PolicyMap; PolicyMap policies_; - bool initialized_; + + // This is used to DCHECK if we try and call IsAllowed or + // FindFirstAllowed before we've finished executing InitFromFile. + // Note: We're "finished" even if loading the file fails -- the + // point of the DCHECK is to make sure we haven't violated our + // ordering/threading assumptions, not to make sure that we're + // properly initialized. + bool init_from_file_finished_; DISALLOW_COPY_AND_ASSIGN(PluginSelectionPolicy); }; diff --git a/chrome/browser/chromeos/plugin_selection_policy_unittest.cc b/chrome/browser/chromeos/plugin_selection_policy_unittest.cc index 455ed37..4bc4538 100644 --- a/chrome/browser/chromeos/plugin_selection_policy_unittest.cc +++ b/chrome/browser/chromeos/plugin_selection_policy_unittest.cc @@ -26,23 +26,23 @@ using std::vector; namespace chromeos { const char kBasicPolicy[] = "# This is a basic policy\n" - "plugin test.so\n" - "allow foo.com\n" - "deny bar.com\n"; + "plugin test.so\n" + "allow foo.com\n" + "deny bar.com\n"; const char kNoPluginPolicy[] = "# This is a policy with missing plugin.\n" - "# Missing plugin test.so\n" - "allow foo.com\n" - "deny bar.com\n"; + "# Missing plugin test.so\n" + "allow foo.com\n" + "deny bar.com\n"; const char kNoRulesPolicy[] = "# This is a policy with no rules\n" - "plugin test.so\n"; + "plugin test.so\n"; const char kEmptyPolicy[] = "# This is an empty policy\n"; const char kCommentTestPolicy[] = "# This is a policy with inline comments.\n" - "plugin test.so# like this\n" - "allow foo.com # and this\n" - "deny bar.com # and this\n"; + "plugin test.so# like this\n" + "allow foo.com # and this\n" + "deny bar.com # and this\n"; const char kMultiPluginTestPolicy[] = "# This is a policy with multiple plugins.\n" @@ -59,9 +59,9 @@ const char kMultiPluginTestPolicy[] = "allow bim.com\n"; const char kWhitespaceTestPolicy[] = "# This is a policy with odd whitespace.\n" - " plugin\ttest.so# like this\n" - "\n\n \n allow\t\tfoo.com # and this\n" - "\tdeny bar.com\t\t\t# and this \n"; + " plugin\ttest.so# like this\n" + "\n\n \n allow\t\tfoo.com # and this\n" + "\tdeny bar.com\t\t\t# and this \n"; class PluginSelectionPolicyTest : public PlatformTest { public: @@ -79,14 +79,19 @@ class PluginSelectionPolicyTest : public PlatformTest { bool CreatePolicy(const std::string& name, const std::string& contents, FilePath* path) { - FilePath policy_file(temp_dir_.path()); - policy_file = policy_file.Append(FilePath(name)); - int bytes_written = file_util::WriteFile(policy_file, - contents.c_str(), - contents.size()); + FilePath policy_file = GetPolicyPath(name); + size_t bytes_written = file_util::WriteFile(policy_file, + contents.c_str(), + contents.size()); if (path) *path = policy_file; - return bytes_written >= 0; + + return bytes_written == contents.size(); + } + + FilePath GetPolicyPath(const std::string& name) { + FilePath policy_file(temp_dir_.path()); + return policy_file.Append(FilePath(name)); } private: @@ -160,37 +165,48 @@ TEST_F(PluginSelectionPolicyTest, IsAllowed) { scoped_refptr<PluginSelectionPolicy> policy1 = new PluginSelectionPolicy; ASSERT_TRUE(policy1->InitFromFile(path)); EXPECT_TRUE(policy1->IsAllowed(GURL("http://www.foo.com/blah.html"), - FilePath("/usr/local/bin/test.so"))); + FilePath("/usr/local/bin/test.so"))); EXPECT_FALSE(policy1->IsAllowed(GURL("http://www.bar.com/blah.html"), - FilePath("/usr/local/bin/test.so"))); + FilePath("/usr/local/bin/test.so"))); EXPECT_FALSE(policy1->IsAllowed(GURL("http://www.baz.com/blah.html"), - FilePath("/usr/local/bin/test.so"))); + FilePath("/usr/local/bin/test.so"))); EXPECT_TRUE(policy1->IsAllowed(GURL("http://www.baz.com/blah.html"), - FilePath("/usr/local/bin/real.so"))); + FilePath("/usr/local/bin/real.so"))); scoped_refptr<PluginSelectionPolicy> policy2 = new PluginSelectionPolicy; ASSERT_TRUE(CreatePolicy("no_rules", kNoRulesPolicy, &path)); ASSERT_TRUE(policy2->InitFromFile(path)); EXPECT_FALSE(policy2->IsAllowed(GURL("http://www.foo.com/blah.html"), - FilePath("/usr/local/bin/test.so"))); + FilePath("/usr/local/bin/test.so"))); EXPECT_FALSE(policy2->IsAllowed(GURL("http://www.bar.com/blah.html"), - FilePath("/usr/local/bin/test.so"))); + FilePath("/usr/local/bin/test.so"))); EXPECT_FALSE(policy2->IsAllowed(GURL("http://www.baz.com/blah.html"), - FilePath("/usr/local/bin/test.so"))); + FilePath("/usr/local/bin/test.so"))); EXPECT_TRUE(policy2->IsAllowed(GURL("http://www.baz.com/blah.html"), - FilePath("/usr/local/bin/real.so"))); + FilePath("/usr/local/bin/real.so"))); scoped_refptr<PluginSelectionPolicy> policy3 = new PluginSelectionPolicy; ASSERT_TRUE(CreatePolicy("empty", kEmptyPolicy, &path)); ASSERT_TRUE(policy3->InitFromFile(path)); EXPECT_TRUE(policy3->IsAllowed(GURL("http://www.foo.com/blah.html"), - FilePath("/usr/local/bin/test.so"))); + FilePath("/usr/local/bin/test.so"))); EXPECT_TRUE(policy3->IsAllowed(GURL("http://www.bar.com/blah.html"), - FilePath("/usr/local/bin/test.so"))); + FilePath("/usr/local/bin/test.so"))); EXPECT_TRUE(policy3->IsAllowed(GURL("http://www.baz.com/blah.html"), - FilePath("/usr/local/bin/test.so"))); + FilePath("/usr/local/bin/test.so"))); EXPECT_TRUE(policy3->IsAllowed(GURL("http://www.baz.com/blah.html"), - FilePath("/usr/local/bin/real.so"))); + FilePath("/usr/local/bin/real.so"))); +} + +TEST_F(PluginSelectionPolicyTest, MissingFile) { + // Don't create any policy file, just get the path to one. + FilePath path = GetPolicyPath("missing_file"); + scoped_refptr<PluginSelectionPolicy> policy = new PluginSelectionPolicy; + ASSERT_FALSE(policy->InitFromFile(path)); + EXPECT_TRUE(policy->IsAllowed(GURL("http://www.foo.com/blah.html"), + FilePath("/usr/local/bin/allow_foo.so"))); + EXPECT_TRUE(policy->IsAllowed(GURL("http://www.bar.com/blah.html"), + FilePath("/usr/local/bin/allow_foo.so"))); } TEST_F(PluginSelectionPolicyTest, FindFirstAllowed) { @@ -199,33 +215,33 @@ TEST_F(PluginSelectionPolicyTest, FindFirstAllowed) { scoped_refptr<PluginSelectionPolicy> policy = new PluginSelectionPolicy; ASSERT_TRUE(policy->InitFromFile(path)); EXPECT_TRUE(policy->IsAllowed(GURL("http://www.foo.com/blah.html"), - FilePath("/usr/local/bin/allow_foo.so"))); - EXPECT_FALSE(policy->IsAllowed(GURL("http://www.bar.com/blah.html"), FilePath("/usr/local/bin/allow_foo.so"))); + EXPECT_FALSE(policy->IsAllowed(GURL("http://www.bar.com/blah.html"), + FilePath("/usr/local/bin/allow_foo.so"))); EXPECT_FALSE(policy->IsAllowed(GURL("http://www.baz.com/blah.html"), - FilePath("/usr/local/bin/allow_foo.so"))); + FilePath("/usr/local/bin/allow_foo.so"))); EXPECT_FALSE(policy->IsAllowed(GURL("http://www.bim.com/blah.html"), - FilePath("/usr/local/bin/allow_foo.so"))); + FilePath("/usr/local/bin/allow_foo.so"))); EXPECT_FALSE(policy->IsAllowed(GURL("http://www.foo.com/blah.html"), - FilePath("/usr/local/bin/allow_baz_bim1.so"))); + FilePath("/usr/local/bin/allow_baz_bim1.so"))); EXPECT_FALSE(policy->IsAllowed(GURL("http://www.bar.com/blah.html"), - FilePath("/usr/local/bin/allow_baz_bim1.so"))); + FilePath("/usr/local/bin/allow_baz_bim1.so"))); EXPECT_TRUE(policy->IsAllowed(GURL("http://www.baz.com/blah.html"), - FilePath("/usr/local/bin/allow_baz_bim1.so"))); + FilePath("/usr/local/bin/allow_baz_bim1.so"))); EXPECT_TRUE(policy->IsAllowed(GURL("http://www.bim.com/blah.html"), - FilePath("/usr/local/bin/allow_baz_bim1.so"))); - EXPECT_FALSE(policy->IsAllowed(GURL("http://www.google.com/blah.html"), FilePath("/usr/local/bin/allow_baz_bim1.so"))); + EXPECT_FALSE(policy->IsAllowed(GURL("http://www.google.com/blah.html"), + FilePath("/usr/local/bin/allow_baz_bim1.so"))); EXPECT_FALSE(policy->IsAllowed(GURL("http://www.foo.com/blah.html"), - FilePath("/usr/local/bin/allow_baz_bim2.so"))); + FilePath("/usr/local/bin/allow_baz_bim2.so"))); EXPECT_FALSE(policy->IsAllowed(GURL("http://www.bar.com/blah.html"), - FilePath("/usr/local/bin/allow_baz_bim2.so"))); + FilePath("/usr/local/bin/allow_baz_bim2.so"))); EXPECT_TRUE(policy->IsAllowed(GURL("http://www.baz.com/blah.html"), - FilePath("/usr/local/bin/allow_baz_bim2.so"))); + FilePath("/usr/local/bin/allow_baz_bim2.so"))); EXPECT_TRUE(policy->IsAllowed(GURL("http://www.bim.com/blah.html"), - FilePath("/usr/local/bin/allow_baz_bim2.so"))); - EXPECT_FALSE(policy->IsAllowed(GURL("http://www.google.com/blah.html"), FilePath("/usr/local/bin/allow_baz_bim2.so"))); + EXPECT_FALSE(policy->IsAllowed(GURL("http://www.google.com/blah.html"), + FilePath("/usr/local/bin/allow_baz_bim2.so"))); std::vector<WebPluginInfo> info_vector; WebPluginInfo info; // First we test that the one without any policy gets @@ -239,13 +255,13 @@ TEST_F(PluginSelectionPolicyTest, FindFirstAllowed) { info.path = FilePath("/usr/local/bin/allow_baz_bim2.so"); info_vector.push_back(info); EXPECT_EQ(0, policy->FindFirstAllowed(GURL("http://www.baz.com/blah.html"), - info_vector)); + info_vector)); EXPECT_EQ(0, policy->FindFirstAllowed(GURL("http://www.foo.com/blah.html"), - info_vector)); + info_vector)); EXPECT_EQ(0, policy->FindFirstAllowed(GURL("http://www.bling.com/blah.html"), - info_vector)); + info_vector)); EXPECT_EQ(0, policy->FindFirstAllowed(GURL("http://www.google.com/blah.html"), - info_vector)); + info_vector)); // Now move the plugin without any policy to the end. info_vector.clear(); @@ -258,13 +274,13 @@ TEST_F(PluginSelectionPolicyTest, FindFirstAllowed) { info.path = FilePath("/usr/local/bin/no_policy.so"); info_vector.push_back(info); EXPECT_EQ(1, policy->FindFirstAllowed(GURL("http://www.baz.com/blah.html"), - info_vector)); + info_vector)); EXPECT_EQ(0, policy->FindFirstAllowed(GURL("http://www.foo.com/blah.html"), - info_vector)); + info_vector)); EXPECT_EQ(3, policy->FindFirstAllowed(GURL("http://www.bling.com/blah.html"), - info_vector)); + info_vector)); EXPECT_EQ(3, policy->FindFirstAllowed(GURL("http://www.google.com/blah.html"), - info_vector)); + info_vector)); } } // namespace chromeos |