Bug 112642 – Assertion in LegacyWebArchive::create() in editing tests

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Simon Fraser (smfr)]

editing/deleting/paste-with-transparent-background-color.html sometimes asserts:
http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20(Tests)/r146140%20(7889)/results.html
http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20(Tests)/r146140%20(7889)/editing/deleting/paste-with-transparent-background-color-crash-log.txt

ASSERTION FAILED: data
/Volumes/Data/slave/mountainlion-debug/build/Source/WebCore/loader/archive/cf/LegacyWebArchive.cpp(269) : static PassRefPtr<WebCore::LegacyWebArchive> WebCore::LegacyWebArchive::create(const WebCore::KURL &, WebCore::SharedBuffer *)
1   0x10a39652a WebCore::LegacyWebArchive::create(WebCore::KURL const&, WebCore::SharedBuffer*)
2   0x10a4ecadc WebCore::Pasteboard::documentFragment(WebCore::Frame*, WTF::PassRefPtr<WebCore::Range>, bool, bool&)
3   0x1097ff5f4 WebCore::Editor::pasteWithPasteboard(WebCore::Pasteboard*, bool)
4   0x1097e5db9 WebCore::Editor::paste()
5   0x1097f9b91 WebCore::executePaste(WebCore::Frame*, WebCore::Event*, WebCore::EditorCommandSource, WTF::String const&)
6   0x1097f60c0 WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const
7   0x1095f7d2e WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&)
8   0x109e53266 WebCore::jsDocumentPrototypeFunctionExecCommand(JSC::ExecState*)
9   0x5e9083a01045
10  0x10833dc34 JSC::JITCode::execute(JSC::JSStack*, JSC::ExecState*, JSC::JSGlobalData*)
11  0x10833a6f8 JSC::Interpreter::execute(JSC::ProgramExecutable*, JSC::ExecState*, JSC::JSObject*)
12  0x1081a82f7 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*)
13  0x10a02db02 WebCore::JSMainThreadExecState::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, JSC::JSValue*)
14  0x10a8bdc13 WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld*)
15  0x10a8bdd64 WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&)
16  0x10a8d5dfa WebCore::ScriptElement::executeScript(WebCore::ScriptSourceCode const&)
17  0x10a8d4b8d WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport)
18  0x109b54d1e WebCore::HTMLScriptRunner::runScript(WebCore::Element*, WTF::TextPosition const&)
19  0x109b54af0 WebCore::HTMLScriptRunner::execute(WTF::PassRefPtr<WebCore::Element>, WTF::TextPosition const&)
20  0x109abb338 WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder()
21  0x109abb44f WebCore::HTMLDocumentParser::canTakeNextToken(WebCore::HTMLDocumentParser::SynchronousMode, WebCore::PumpSession&)
22  0x109abab87 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)
23  0x109aba610 WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode)
24  0x109abbb9c WebCore::HTMLDocumentParser::append(WTF::PassRefPtr<WTF::StringImpl>)
25  0x1095b5c9c WebCore::DecodedDataDocumentParser::appendBytes(WebCore::DocumentWriter*, char const*, unsigned long)
26  0x10968a733 WebCore::DocumentWriter::addData(char const*, unsigned long)
27  0x10964f357 WebCore::DocumentLoader::commitData(char const*, unsigned long)
28  0x106e6c1a1 WebKit::WebFrameLoaderClient::committedLoad(WebCore::DocumentLoader*, char const*, int)
29  0x10964fd80 WebCore::DocumentLoader::commitLoad(char const*, int)
30  0x10964eda0 WebCore::DocumentLoader::receivedData(char const*, int)
31  0x10a3b5c42 WebCore::MainResourceLoader::dataReceived(WebCore::CachedResource*, char const*, int)

[Simon Fraser (smfr)]

Also editing/pasteboard/5780697-1.html
http://build.webkit.org/results/Apple%20Lion%20Debug%20WK2%20(Tests)/r146209%20(8187)/editing/pasteboard/5780697-1-crash-log.txt

[Ryosuke Niwa]

Created attachment 194113 [details]
Fixes the bug

[Simon Fraser (smfr)]

Comment on attachment 194113 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=194113&action=review

> Source/WebCore/platform/mac/PasteboardMac.mm:482
> +        return createFragmentFromMarkup(frame->document(), markupString, mainResource->url(), DisallowScriptingAndPluginContent);
> +        [markupString release];

return then release?

[Enrica Casucci]

Comment on attachment 194113 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=194113&action=review

> Source/WebCore/ChangeLog:8
> +        It's possible for clipboard to be overriden between the time we retrieve clipboard types and the time

typo overwritten

> Source/WebCore/platform/mac/PasteboardMac.mm:476
> +        NSString *markupString = [[NSString alloc] initWithData:[mainResource->data()->createNSData() autorelease] encoding:NSUTF8StringEncoding];

Since you're refactoring the code, you could use RetainPtr and avoid the release.

[Ryosuke Niwa]

Created attachment 194116 [details]
Fix that

[Enrica Casucci]

Comment on attachment 194116 [details]
Fix that

View in context: https://bugs.webkit.org/attachment.cgi?id=194116&action=review

Please fix the typo.

> Source/WebCore/ChangeLog:8
> +        It's possible for clipboard to be overriden between the time we retrieve clipboard types and the time

typo overwritten.

> Source/WebCore/platform/mac/PasteboardMac.mm:476
> +        NSString *markupString = [[NSString alloc] initWithData:[mainResource->data()->createNSData() autorelease] encoding:NSUTF8StringEncoding];

You could have used RetainPtr here and avoid calling release.

[Simon Fraser (smfr)]

Comment on attachment 194116 [details]
Fix that

View in context: https://bugs.webkit.org/attachment.cgi?id=194116&action=review

> Source/WebCore/platform/mac/PasteboardMac.mm:475
> +    NSString *MIMEType = mainResource->mimeType();
> +    if (!frame || !frame->document())
> +        return 0;
> +
> +    if (frame->loader()->client()->canShowMIMETypeAsHTML(MIMEType)) {

Is this doing conversions between NSString* and String?

>> Source/WebCore/platform/mac/PasteboardMac.mm:476
>> +        NSString *markupString = [[NSString alloc] initWithData:[mainResource->data()->createNSData() autorelease] encoding:NSUTF8StringEncoding];
> 
> You could have used RetainPtr here and avoid calling release.

There seems little point in avoiding a single autorelease here.

[Ryosuke Niwa]

Landed in http://trac.webkit.org/changeset/146389 and http://trac.webkit.org/changeset/146391.

[Alexey Proskuryakov]

The tests were still marked as crashing, will remove the expectations now.