Only happening on Apple Yosemite Debug WK2. https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=js%2Fdom%2Fnavigator-plugins-crash.html Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x00000001153eee17 WTFCrash + 39 1 com.apple.WebCore 0x0000000116ae5a99 WTF::CrashOnOverflow::overflowed() + 9 2 com.apple.WebCore 0x000000011717d791 WTF::Vector<WebCore::PluginInfo, 0ul, WTF::CrashOnOverflow, 16ul>::at(unsigned long) + 81 (Vector.h:660) 3 com.apple.WebCore 0x000000011717d65d WTF::Vector<WebCore::PluginInfo, 0ul, WTF::CrashOnOverflow, 16ul>::operator[](unsigned long) + 29 (Vector.h:679) 4 com.apple.WebCore 0x000000011717cfdf WebCore::DOMPlugin::pluginInfo() const + 79 (DOMPlugin.cpp:61) 5 com.apple.WebCore 0x000000011717cf67 WebCore::DOMPlugin::name() const + 39 (DOMPlugin.cpp:41) 6 com.apple.WebCore 0x00000001179e6ff7 WebCore::jsDOMPluginName(JSC::ExecState*, JSC::JSObject*, long long, JSC::PropertyName) + 71 (JSDOMPlugin.cpp:240)
I don't know why this started now, but the code is somewhat new too, as it was added in <http://trac.webkit.org/changeset/181562>. I don't think that there is any guarantee of m_index remaining a valid index.
<rdar://problem/20750473>
Marked the test as flakily crashing in r185598.
Created attachment 255023 [details] Patch
Comment on attachment 255023 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255023&action=review > Source/WebCore/plugins/DOMPlugin.cpp:31 > + , m_pluginInfo(pluginInfo) I think this could be WTF::move(pluginInfo) for slightly better performance. > Source/WebCore/plugins/DOMPlugin.h:36 > + static Ref<DOMPlugin> create(PluginData* pluginData, Frame* frame, PluginInfo pluginInfo) { return adoptRef(*new DOMPlugin(pluginData, frame, pluginInfo)); } I think this could use WTF::move(pluginInfo) for slightly better performance. Should take a PluginData& argument instead of PluginData*. > Source/WebCore/plugins/DOMPluginArray.cpp:90 > const Vector<PluginInfo>& plugins = data->webVisiblePlugins(); > for (unsigned i = 0; i < plugins.size(); ++i) { > if (plugins[i].name == propertyName) > - return DOMPlugin::create(data, m_frame, i); > + return DOMPlugin::create(data, m_frame, plugins[i]); > } Should be rewritten as a modern for loop: for (auto& plugin : data->webVisiblePlugins()) { if (plugin.name == propertyName) return DOMPlugin::create(data, m_frame, plugin); } The loop used the old fashioned form only because it wanted an "i" to pass to DOMPlugin::create. > Source/WebCore/plugins/PluginData.h:76 > +inline bool operator==(PluginInfo a, PluginInfo b) Better to take const PluginInfo& arguments instead of copying the PluginInfo objects just to compare them.
Please revert the change in r185598 when this lands.
Comment on attachment 255023 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=255023&action=review Thanks, Darin! >> Source/WebCore/plugins/DOMPlugin.cpp:31 >> + , m_pluginInfo(pluginInfo) > > I think this could be WTF::move(pluginInfo) for slightly better performance. Done. >> Source/WebCore/plugins/DOMPlugin.h:36 >> + static Ref<DOMPlugin> create(PluginData* pluginData, Frame* frame, PluginInfo pluginInfo) { return adoptRef(*new DOMPlugin(pluginData, frame, pluginInfo)); } > > I think this could use WTF::move(pluginInfo) for slightly better performance. > > Should take a PluginData& argument instead of PluginData*. I adopted WTF::move(), but changing PluginData is outside the scope of this bug and has wider ramifications, so I'd prefer to address that separately. >> Source/WebCore/plugins/DOMPluginArray.cpp:90 >> } > > Should be rewritten as a modern for loop: > > for (auto& plugin : data->webVisiblePlugins()) { > if (plugin.name == propertyName) > return DOMPlugin::create(data, m_frame, plugin); > } > > The loop used the old fashioned form only because it wanted an "i" to pass to DOMPlugin::create. Good catch. Fixed. >> Source/WebCore/plugins/PluginData.h:76 >> +inline bool operator==(PluginInfo a, PluginInfo b) > > Better to take const PluginInfo& arguments instead of copying the PluginInfo objects just to compare them. Yup, done.
Committed r185717: <http://trac.webkit.org/changeset/185717>
(In reply to comment #6) > Please revert the change in r185598 when this lands. Done in r185718.