We currently prevent e.g <div style="display: none"> and <span style="display: none"> from sharing attribute data. The reason we do this is because presentation attributes map to different CSS properties depending on the name of the tag they're on. A cool optimization would be to force elements with presentation attributes to always use mutable attribute data. This way we could share equivalent immutable attribute data between all element types, and shrink ImmutableElementAttributeData by one pointer.
Created attachment 175295 [details] Patch
Attachment 175295 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/dom/Element.cpp:2567: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] Source/WebCore/dom/Element.cpp:2568: Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side. [whitespace/operators] [4] Total errors found: 2 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 175295 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175295&action=review > Source/WebCore/ChangeLog:25 > + On the upside, excluding elements with presentation attributes from the data cache means that > + we can move one pointer (m_presentationAttributeStyle) from ImmutableElementAttributeData > + to MutableElementAttributeData. I suppose we could also add 3rd state, ImmutableElementAttributeDataWithPresentationAttributeStyle. Might not be worth it.
Comment on attachment 175295 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175295&action=review >> Source/WebCore/ChangeLog:25 >> + to MutableElementAttributeData. > > I suppose we could also add 3rd state, ImmutableElementAttributeDataWithPresentationAttributeStyle. Might not be worth it. That would be a reasonable way to go if this causes a monstrous regression on that Y2K content chromium tracks.
Committed r135421: <http://trac.webkit.org/changeset/135421>
(In reply to comment #4) > (From update of attachment 175295 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=175295&action=review > > >> Source/WebCore/ChangeLog:25 > >> + to MutableElementAttributeData. > > > > I suppose we could also add 3rd state, ImmutableElementAttributeDataWithPresentationAttributeStyle. Might not be worth it. > > That would be a reasonable way to go if this causes a monstrous regression on that Y2K content chromium tracks. llol. It's funny cuz it's true. We talk about updating it once in a while, but noone's made it a priority. So far, our performance work has mostly gone into doing better about noticing regressions. I expect we'll get around to this eventually. I actually think tracking both old and new context is worthwhile. Anyways, I agree. Lets see if we need it. I think it's unlikely that this will cause a regression in any real content.
This seems to assert editing/undo/replace-by-span-then-remove.html: 11:40:02.722 9037 ASSERTION FAILED: !other.m_inlineStyle->isMutable() 11:40:02.722 9037 /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/Source/WebCore/dom/ElementAttributeData.cpp(76) : WebCore::ImmutableElementAttributeData::ImmutableElementAttributeData(const WebCore::MutableElementAttributeData&) 11:40:02.722 9037 1 0x7f4b3da4ada6 WebCore::ImmutableElementAttributeData::ImmutableElementAttributeData(WebCore::MutableElementAttributeData const&) 11:40:02.722 9037 2 0x7f4b3da4b2ff WebCore::ElementAttributeData::makeImmutableCopy() const 11:40:02.722 9037 3 0x7f4b3da41e5b WebCore::Element::cloneAttributesFromElement(WebCore::Element const&) 11:40:02.722 9037 4 0x7f4b3da41fc1 WebCore::Element::cloneDataFromElement(WebCore::Element const&) 11:40:02.722 9037 5 0x7f4b3db62171 11:40:02.722 9037 6 0x7f4b3db622c5 WebCore::ReplaceNodeWithSpanCommand::doApply() 11:40:02.722 9037 7 0x7f4b3db1d569 WebCore::SimpleEditCommand::doReapply() 11:40:02.722 9037 8 0x7f4b3db03c42 WebCore::EditCommandComposition::reapply() 11:40:02.722 9037 9 0x7f4b41f20a67 WebKit::WebPage::reapplyEditCommand(unsigned long) 11:40:02.722 9037 10 0x7f4b41fe5cc0 void CoreIPC::callMemberFunction<WebKit::WebPage, void (WebKit::WebPage::*)(unsigned long), unsigned long>(CoreIPC::Arguments1<unsigned long> const&, WebKit::WebPage*, void (WebKit::WebPage::*)(unsigned long)) 11:40:02.722 9037 11 0x7f4b41fe44d3 void CoreIPC::handleMessage<Messages::WebPage::ReapplyEditCommand, WebKit::WebPage, void (WebKit::WebPage::*)(unsigned long)>(CoreIPC::MessageDecoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(unsigned long)) 11:40:02.722 9037 12 0x7f4b41fe04fe WebKit::WebPage::didReceiveWebPageMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&) 11:40:02.722 9037 13 0x7f4b41f2171a WebKit::WebPage::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&) 11:40:02.722 9037 14 0x7f4b41cecedd CoreIPC::MessageReceiverMap::dispatchMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&) 11:40:02.722 9037 15 0x7f4b41e4e208 WebKit::WebProcess::didReceiveMessage(CoreIPC::Connection*, CoreIPC::MessageID, CoreIPC::MessageDecoder&) 11:40:02.722 9037 16 0x7f4b41ce1e64 CoreIPC::Connection::dispatchMessage(CoreIPC::MessageID, CoreIPC::MessageDecoder&) 11:40:02.722 9037 17 0x7f4b41ce1fd0 CoreIPC::Connection::dispatchMessage(CoreIPC::Connection::Message<CoreIPC::MessageDecoder>&) 11:40:02.722 9037 18 0x7f4b41cdfd07 CoreIPC::Connection::SyncMessageState::dispatchMessages() 11:40:02.722 9037 19 0x7f4b41ce1149 CoreIPC::Connection::waitForSyncReply(unsigned long, double, unsigned int) 11:40:02.722 9037 20 0x7f4b41ce0fb0 CoreIPC::Connection::sendSyncMessage(CoreIPC::MessageID, unsigned long, WTF::PassOwnPtr<CoreIPC::MessageEncoder>, double, unsigned int) 11:40:02.723 9037 21 0x7f4b41ee3780 bool CoreIPC::Connection::sendSync<Messages::WebPageProxy::ExecuteUndoRedo>(Messages::WebPageProxy::ExecuteUndoRedo const&, Messages::WebPageProxy::ExecuteUndoRedo::Reply const&, unsigned long, double, unsigned int) 11:40:02.723 9037 22 0x7f4b41ee2b10 bool CoreIPC::MessageSender<WebKit::WebPage>::sendSync<Messages::WebPageProxy::ExecuteUndoRedo>(Messages::WebPageProxy::ExecuteUndoRedo const&, Messages::WebPageProxy::ExecuteUndoRedo::Reply const&, unsigned long, double) 11:40:02.723 9037 23 0x7f4b41ee2307 bool CoreIPC::MessageSender<WebKit::WebPage>::sendSync<Messages::WebPageProxy::ExecuteUndoRedo>(Messages::WebPageProxy::ExecuteUndoRedo const&, Messages::WebPageProxy::ExecuteUndoRedo::Reply const&, double) 11:40:02.723 9037 24 0x7f4b41ee0c23 WebKit::WebEditorClient::redo() 11:40:02.723 9037 25 0x7f4b3db2cbce WebCore::Editor::redo() 11:40:02.723 9037 26 0x7f4b3db3d77e 11:40:02.723 9037 27 0x7f4b3db3f389 WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const 11:40:02.723 9037 28 0x7f4b3d9dd9da WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) 11:40:02.723 9037 29 0x7f4b3e69cf7d WebCore::jsDocumentPrototypeFunctionExecCommand(JSC::ExecState*) 11:40:02.723 9037 30 0x7f4aed3ed265