Consider the frame structure in Matches()

test/FilterEngine.cpp

Index: test/FilterEngine.cpp
===================================================================
--- a/test/FilterEngine.cpp
+++ b/test/FilterEngine.cpp
@@ -202,6 +202,72 @@
   ASSERT_EQ(AdblockPlus::Filter::TYPE_BLOCKING, match12->GetType());
 }
 
+TEST_F(FilterEngineTest, MatchesOnWhitelistedDomain)
+{
+  filterEngine->GetFilter("adbanner.gif")->AddToList();
+  filterEngine->GetFilter("@@||example.org$document")->AddToList();

[Wladimir Palant, 2013/11/04 07:17:28]

Nit: This should be @@||example.org^$document (^ is a separator placeholder - it
makes sure the domain name actually ends here).

[Felix Dahlke, 2013/11/15 08:18:19]

Done.

+
+  AdblockPlus::FilterPtr match1 =
+    filterEngine->Matches("http://ads.com/adbanner.gif", "IMAGE",
+                          "http://example.com/");
+  ASSERT_TRUE(match1);
+  ASSERT_EQ(AdblockPlus::Filter::TYPE_BLOCKING, match1->GetType());
+
+  AdblockPlus::FilterPtr match2 =
+    filterEngine->Matches("http://ads.com/adbanner.gif", "IMAGE",
+                          "http://example.org/");
+  ASSERT_TRUE(match2);
+  ASSERT_EQ(AdblockPlus::Filter::TYPE_EXCEPTION, match2->GetType());
+}
+
+TEST_F(FilterEngineTest, MatchesNestedFrameRequest)
+{
+  filterEngine->GetFilter("adbanner.gif")->AddToList();
+  filterEngine->GetFilter("@@adbanner.gif$domain=example.org")->AddToList();
+
+  std::vector<std::string> documentUrls1;
+  documentUrls1.push_back("http://ads.com/frame/");
+  documentUrls1.push_back("http://example.com/");
+  AdblockPlus::FilterPtr match1 =
+    filterEngine->Matches("http://ads.com/adbanner.gif", "IMAGE",
+                          documentUrls1);
+  ASSERT_TRUE(match1);
+  ASSERT_EQ(AdblockPlus::Filter::TYPE_BLOCKING, match1->GetType());
+
+  std::vector<std::string> documentUrls2;
+  documentUrls2.push_back("http://ads.com/frame/");
+  documentUrls2.push_back("http://example.org/");
+  AdblockPlus::FilterPtr match2 =
+    filterEngine->Matches("http://ads.com/adbanner.gif", "IMAGE",
+                          documentUrls2);

[Wladimir Palant, 2013/11/04 07:17:28]

How about an additional test where the document URLs are http://example.org/ and
http://ads.com/frame/ (in reverse order)? This should return a blocking filter
and didn't with your original implementation.

[Felix Dahlke, 2013/11/15 08:18:19]

Done.

+  ASSERT_TRUE(match2);
+  ASSERT_EQ(AdblockPlus::Filter::TYPE_EXCEPTION, match2->GetType());
+}
+
+TEST_F(FilterEngineTest, MatchesNestedFrameOnWhitelistedDomain)
+{
+  filterEngine->GetFilter("adbanner.gif")->AddToList();
+  filterEngine->GetFilter("@@||example.org$document")->AddToList();

[Wladimir Palant, 2013/11/04 07:17:28]

Nit: As above, this should be @@||example.org^$document. However, I would prefer
testing @@||example.org^$document,domain=ads.com to make sure that the parent
domain is also checked properly.

[Felix Dahlke, 2013/11/15 08:18:19]

Done.

+
+  std::vector<std::string> documentUrls1;
+  documentUrls1.push_back("http://ads.com/frame/");
+  documentUrls1.push_back("http://example.com/");
+  AdblockPlus::FilterPtr match1 =
+    filterEngine->Matches("http://ads.com/adbanner.gif", "IMAGE",
+                          documentUrls1);
+  ASSERT_TRUE(match1);
+  ASSERT_EQ(AdblockPlus::Filter::TYPE_BLOCKING, match1->GetType());
+
+  std::vector<std::string> documentUrls2;
+  documentUrls2.push_back("http://ads.com/frame/");
+  documentUrls2.push_back("http://example.org/");
+  AdblockPlus::FilterPtr match2 =
+    filterEngine->Matches("http://ads.com/adbanner.gif", "IMAGE",
+                          documentUrls2);
+  ASSERT_TRUE(match2);
+  ASSERT_EQ(AdblockPlus::Filter::TYPE_EXCEPTION, match2->GetType());

[Wladimir Palant, 2013/11/04 07:17:28]

I would add a few more tests here:

* http://example.org/ only - should return TYPE_BLOCKING.
* http://example.org/, http://ads.com/frame/ - should return TYPE_BLOCKING.
* http://ads.com/frame/, http://example.org/, http://example.com/ - should
return TYPE_EXCEPTION.

[Felix Dahlke, 2013/11/15 08:18:19]

Done.

+}
+
 TEST_F(FilterEngineTest, FirstRunFlag)
 {
   ASSERT_FALSE(filterEngine->IsFirstRun());