Issue 3363 - Implement IsDocumentWhitelisted and IsElemhideWhitelisted

src/FilterEngine.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 310 matching lines @@
       return match;
     lastDocumentUrl = documentUrl;
   }
 
   return CheckFilterMatch(url, contentType, lastDocumentUrl);
 }
 
 bool FilterEngine::IsDocumentWhitelisted(const std::string& url,
     const std::vector<std::string>& documentUrls) const
 {
-    return GetWhitelistingFilter(url, CONTENT_TYPE_DOCUMENT, documentUrls) != 0;

[Felix Dahlke, 2015/12/02 18:02:03]

Nit: != 0 is redundant, according to the Mozilla coding style it should be left
out. (Just assuming that actually compiles and I'm not missing something here,
I'm getting a bit rusty.)

Same below.

[René Jeschke, 2015/12/02 18:09:42]

I know, and I wouldn't normally do this (as you know), but this time I insist on
having the compare here, as we're returning a 'bool' and it feels/looks wrong
returning an object instead. :-)

[sergei, 2015/12/02 18:21:57]

Let me take a part, I would compare with `nullptr` instead of `0` or use `!!` to
indicate that we are adapting it to bool.
BTW, having `explicit operator bool` of `FilterPtr` will cause a compilation
error if we try to return in instance of `FilterPtr`.

[Felix Dahlke, 2015/12/02 18:50:26]

Yeah, !! is pretty idiomatic for coercing an arbitrary value to boolean.

[René Jeschke, 2015/12/03 11:31:20]

Yep, fine with me. Went with !! in the end.

+    return !!GetWhitelistingFilter(url, CONTENT_TYPE_DOCUMENT, documentUrls);
 }
 
 bool FilterEngine::IsElemhideWhitelisted(const std::string& url,
     const std::vector<std::string>& documentUrls) const
 {
-    return GetWhitelistingFilter(url, CONTENT_TYPE_ELEMHIDE, documentUrls) != 0;
+    return !!GetWhitelistingFilter(url, CONTENT_TYPE_ELEMHIDE, documentUrls);
 }
 
 AdblockPlus::FilterPtr FilterEngine::CheckFilterMatch(const std::string& url,
     ContentType contentType,
     const std::string& documentUrl) const
 {
   JsValuePtr func = jsEngine->Evaluate("API.checkFilterMatch");
   JsValueList params;
   params.push_back(jsEngine->NewValue(url));
   params.push_back(jsEngine->NewValue(ContentTypeToString(contentType)));
@@ skipping 124 matching lines @@
   params.push_back(jsEngine->NewValue(v1));
   params.push_back(jsEngine->NewValue(v2));
   JsValuePtr func = jsEngine->Evaluate("API.compareVersions");
   return func->Call(params)->AsInt();
 }
 
 FilterPtr FilterEngine::GetWhitelistingFilter(const std::string& url,
   ContentType contentType, const std::string& documentUrl) const
 {
   FilterPtr match = Matches(url, contentType, documentUrl);
-

[Felix Dahlke, 2015/12/02 18:02:03]

Micro nit: Superfluous whitespace? Can't tell if it's deliberate or not.

[René Jeschke, 2015/12/02 18:09:42]

Done.

   if (match && match->GetType() == Filter::TYPE_EXCEPTION)
   {
     return match;
   }
   return FilterPtr();
 }
 
 FilterPtr FilterEngine::GetWhitelistingFilter(const std::string& url,
   ContentType contentType,
   const std::vector<std::string>& documentUrls) const
@@ skipping 12 matching lines @@
                          currentUrl, contentType, parentUrl);
     if (filter)
     {
       return filter;
     }
     currentUrl = parentUrl;
   }
   while (urlIterator != documentUrls.end());
   return FilterPtr();
 }