Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: src/plugin/PluginFilter.cpp

Issue 5070706781978624: Issue #276 - introduce class BSTR_Argument (Closed)
Patch Set: Created July 25, 2014, 9:32 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« src/plugin/PluginClass.cpp ('K') | « src/plugin/PluginFilter.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/plugin/PluginFilter.cpp
===================================================================
--- a/src/plugin/PluginFilter.cpp
+++ b/src/plugin/PluginFilter.cpp
@@ -2,6 +2,7 @@
#include "PluginFilter.h"
+#include "COM_Value.h"
#if (defined PRODUCT_ADBLOCKPLUS)
#include "PluginSettings.h"
#include "PluginClient.h"
@@ -16,7 +17,6 @@
#include "..\shared\CriticalSection.h"
-
// The filters are described at http://adblockplus.org/en/filters
static CriticalSection s_criticalSectionFilterMap;
@@ -25,7 +25,7 @@
// CFilterElementHideAttrSelector
// ============================================================================
-CFilterElementHideAttrSelector::CFilterElementHideAttrSelector() : m_type(TYPE_NONE), m_pos(POS_NONE), m_bstrAttr(NULL)
+CFilterElementHideAttrSelector::CFilterElementHideAttrSelector() : m_type(TYPE_NONE), m_pos(POS_NONE), m_attr(L"")
sergei 2014/07/28 09:18:26 CString should have a default constructor, just re
Eric 2014/07/29 16:59:27 Done.
{
}
@@ -33,7 +33,7 @@
{
m_type = filter.m_type;
m_pos = filter.m_pos;
- m_bstrAttr = filter.m_bstrAttr;
+ m_attr = filter.m_attr;
m_value = filter.m_value;
}
@@ -150,26 +150,26 @@
if (arg.GetAt(delimiterPos - 1) == '^')
{
- attrSelector.m_bstrAttr = arg.Left(delimiterPos - 1);
+ attrSelector.m_attr = arg.Left(delimiterPos - 1);
attrSelector.m_pos = CFilterElementHideAttrPos::STARTING;
}
else if (arg.GetAt(delimiterPos - 1) == '*')
{
- attrSelector.m_bstrAttr = arg.Left(delimiterPos - 1);
+ attrSelector.m_attr = arg.Left(delimiterPos - 1);
attrSelector.m_pos = CFilterElementHideAttrPos::ANYWHERE;
}
else if (arg.GetAt(delimiterPos - 1) == '$')
{
- attrSelector.m_bstrAttr = arg.Left(delimiterPos - 1);
+ attrSelector.m_attr = arg.Left(delimiterPos - 1);
attrSelector.m_pos = CFilterElementHideAttrPos::ENDING;
}
else
{
- attrSelector.m_bstrAttr = arg.Left(delimiterPos);
+ attrSelector.m_attr = arg.Left(delimiterPos);
attrSelector.m_pos = CFilterElementHideAttrPos::EXACT;
}
}
- CString tag = attrSelector.m_bstrAttr;
+ CString tag = attrSelector.m_attr;
if (tag == "style")
{
attrSelector.m_type = CFilterElementHideAttrType::STYLE;
@@ -242,20 +242,20 @@
if (!m_tagId.IsEmpty())
{
- CComBSTR id;
+ AdblockPlus::COM::BSTR_Argument id;
hr = pEl->get_id(&id);
- if ((hr != S_OK) || (id != CComBSTR(m_tagId)))
+ if ((hr != S_OK) || (to_CString(id) != m_tagId))
{
return false;
}
}
if (!m_tagClassName.IsEmpty())
{
- CComBSTR classNameBSTR;
- hr = pEl->get_className(&classNameBSTR);
+ AdblockPlus::COM::BSTR_Argument result;
+ hr = pEl->get_className(&result);
if (hr == S_OK)
{
- CString className = classNameBSTR;
+ CString className = to_CString(result);
int start = 0;
CString specificClass;
bool foundMatch = false;
@@ -275,10 +275,11 @@
}
if (!m_tag.IsEmpty())
{
- CComBSTR tagName;
- tagName.ToLower();
Eric 2014/07/25 21:42:16 This was pretty obviously a defect.
- hr = pEl->get_tagName(&tagName);
- if ((hr != S_OK) || (tagName != CComBSTR(m_tag)))
+ AdblockPlus::COM::BSTR_Argument result;
+ hr = pEl->get_tagName(&result);
+ CString tagName = to_CString(result);
+ tagName.MakeLower();
+ if ((hr != S_OK) || tagName != m_tag)
{
return false;
}
@@ -295,38 +296,47 @@
CComPtr<IHTMLStyle> pStyle;
if (SUCCEEDED(pEl->get_style(&pStyle)) && pStyle)
{
- CComBSTR bstrStyle;
-
- if (SUCCEEDED(pStyle->get_cssText(&bstrStyle)) && bstrStyle)
+ AdblockPlus::COM::BSTR_Argument cssText;
+ if (SUCCEEDED(pStyle->get_cssText(&cssText)))
{
- value = bstrStyle;
- value.MakeLower();
- attrFound = true;
+ value = to_CString(cssText);
+ if (!value.IsEmpty())
+ {
+ value.MakeLower();
+ attrFound = true;
+ }
}
}
}
else if (attrIt->m_type == CFilterElementHideAttrType::CLASS)
{
- CComBSTR bstrClassNames;
- if (SUCCEEDED(pEl->get_className(&bstrClassNames)) && bstrClassNames)
+ AdblockPlus::COM::BSTR_Argument classNames;
+ if (SUCCEEDED(pEl->get_className(&classNames)))
{
- value = bstrClassNames;
- attrFound = true;
+ value = to_CString(classNames);
+ if (!value.IsEmpty())
+ {
+ attrFound = true;
+ }
}
}
else if (attrIt->m_type == CFilterElementHideAttrType::ID)
{
- CComBSTR bstrId;
- if (SUCCEEDED(pEl->get_id(&bstrId)) && bstrId)
+ AdblockPlus::COM::BSTR_Argument id;
+ if (SUCCEEDED(pEl->get_id(&id)))
{
- value = bstrId;
- attrFound = true;
+ value = to_CString(id);
+ if (!value.IsEmpty())
+ {
+ attrFound = true;
+ }
}
}
else
{
CComVariant vAttr;
- if (SUCCEEDED(pEl->getAttribute(attrIt->m_bstrAttr, 0, &vAttr)))
+ AdblockPlus::COM::BSTR_Argument attribute_name(to_wstring(attrIt->m_attr));
+ if (SUCCEEDED(pEl->getAttribute(attribute_name, 0, &vAttr)))
{
attrFound = true;
if (vAttr.vt == VT_BSTR)
@@ -507,17 +517,17 @@
bool CPluginFilter::IsElementHidden(const CString& tag, IHTMLElement* pEl, const CString& domain, const CString& indent) const
{
CString id;
- CComBSTR bstrId;
- if (SUCCEEDED(pEl->get_id(&bstrId)) && bstrId)
+ AdblockPlus::COM::BSTR_Argument result;
+ if (SUCCEEDED(pEl->get_id(&result)))
{
- id = bstrId;
+ id = to_CString(result);
}
CString classNames;
- CComBSTR bstrClassNames;
- if (SUCCEEDED(pEl->get_className(&bstrClassNames)) && bstrClassNames)
+ // reuse of 'result' is not an error, since address-of operator permits it
sergei 2014/07/28 09:18:26 It's too specific and requires additional comment
Eric 2014/07/29 16:59:27 It will also disappear by itself, since these are
sergei 2014/07/30 10:10:54 Sorry, not clear.
Eric 2014/07/30 14:14:42 What is your question?
+ if (SUCCEEDED(pEl->get_className(&result)))
{
- classNames = bstrClassNames;
+ classNames = to_CString(result);
}
CriticalSection::Lock filterEngineLock(s_criticalSectionFilterMap);
« src/plugin/PluginClass.cpp ('K') | « src/plugin/PluginFilter.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld