[webkit-reviews] review denied: [Bug 101274] Allow plugins to be disabled by shared library filename : [Attachment 174542] Patch

[bugzilla-daemon at webkit.org]

Yong Li <yoli at rim.com> has denied Max Feil <mfeil at rim.com>'s request for
review:
Bug 101274: Allow plugins to be disabled by shared library filename
https://bugs.webkit.org/show_bug.cgi?id=101274

Attachment 174542: Patch
https://bugs.webkit.org/attachment.cgi?id=174542&action=review

------- Additional Comments from Yong Li <yoli at rim.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=174542&action=review


>>>> Source/WebCore/plugins/PluginDatabase.cpp:324
>>>> +	  HashSet<String>::const_iterator filesEnd =
m_disabledPluginFiles.end();
>>>> +	  HashSet<String>::const_iterator it;
>>>> +	  for (it = m_disabledPluginFiles.begin(); it != filesEnd; ++it) {
>>>> +	      if (fileName.endsWith(*it))
>>>> +		  break;
>>> 
>>> Why endsWith()? Also I would return false here rather than checking
it==filesEnd
>> 
>> The "endsWith" is so that we don't have to specify an entire path name, just
the important part of the path which is at the end. It's probably better to
isolate everything after the last slash for comparison though...
> 
> You can use pathGetFileName(). See WebCore/platform/FileSystem.h
> 
> Also I think checking the disabledPlugin list first may be faster in most
cases. fileExists() might be slow?

If fileName can be a path, we can use pathGetFileName. Then we just need
"return !m_disabledPluginFiles.contain(fileName)". That is also faster than
walking through the list.