Bug 108984

Created attachment 186712 [details] work in progress

Comment on attachment 186712 [details] work in progress I'm not sure if it makes sense for the parser to extend the document's lifetime or not. Another approch is to put this off onto its own separate ref-counted object. But it seems reasonable to me. We could just make processingData() not be public, and instead have a new Document::parsingInProgress() or something which checks both of these on Document. I think that woudl be better abstraction than DocumetnLoader knowing about DocumetnParser.

> I'm not sure if it makes sense for the parser to extend the document's lifetime or not. Another approch is to put this off onto its own separate ref-counted object. I see. The document loader would find the object from the document in isLoadingInAPISense. Yeah, that seems better than refing the document. By the way, this patch doesn't work yet. It seems to delay the load event forever in this test case: <script> if (window.testRunner) { testRunner.dumpAsText(); } window.onload = function() { alert(42); }; </script> <iframe onload="alert(0);document.write('<p>PASS</p>');alert(2);document.close();alert(3);"></iframe> FAIL

Created attachment 187167 [details] passes foo.html test case

Comment on attachment 187167 [details] passes foo.html test case View in context: https://bugs.webkit.org/attachment.cgi?id=187167&action=review > Source/WebCore/dom/Document.cpp:5781 > + if (!frame()->loader()->stateMachine()->creatingInitialEmptyDocument()) WAT? > Source/WebCore/html/parser/HTMLParserScheduler.cpp:65 > + : document(doc) m_document? I'm sure this is private. It's not clear to me if PumpSession should really hold a Document*. > Source/WebCore/loader/DocumentLoader.cpp:485 > + if (doc->parserSessionNestingLevel() > 0) > + return true; > if (DocumentParser* parser = doc->parser()) > if (parser->processingData()) > return true; I would combine these all into one if, moving the processingData() function onto Document. Not surew ht the name is. isParsing()? parserIsActive?

Created attachment 187176 [details] Patch

Comment on attachment 187176 [details] Patch This looks fantastic.

Comment on attachment 187176 [details] Patch Rejecting attachment 187176 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 187176, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: at 305. Hunk #2 succeeded at 365 (offset 9 lines). 1 out of 2 hunks FAILED -- saving rejects to file Source/WebCore/html/parser/HTMLDocumentParser.cpp.rej patching file Source/WebCore/html/parser/HTMLParserScheduler.cpp patching file Source/WebCore/html/parser/HTMLParserScheduler.h patching file Source/WebCore/loader/DocumentLoader.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Eric Seidel']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/16429390

Created attachment 187198 [details] Patch for landing

Comment on attachment 187198 [details] Patch for landing Rejecting attachment 187198 [details] from commit-queue. New failing tests: perf/nested-combined-selectors.html perf/accessibility-title-ui-element.html perf/document-contains.html fast/parser/empty-text-resource.html Full output: http://queues.webkit.org/results/16426462

Created attachment 187418 [details] Patch for landing

Comment on attachment 187418 [details] Patch for landing Clearing flags on attachment: 187418 Committed r142378: <http://trac.webkit.org/changeset/142378>

All reviewed patches have been landed. Closing bug.

This broke the Apple Windows build: http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/62203 7>WebCore.lib(DocumentLoader.obj) : error LNK2001: unresolved external symbol "public: bool __thiscall WebCore::Document::hasActiveParser(void)" (?hasActiveParser@Document@WebCore@@QAE_NXZ) 7>WebCore.lib(HTMLParserScheduler.obj) : error LNK2001: unresolved external symbol "public: void __thiscall WebCore::Document::decrementActiveParserCount(void)" (?decrementActiveParserCount@Document@WebCore@@QAEXXZ)

ah. Export files. :)

Sorry for breaking the build. We probably just need to add ?decrementActiveParserCount@Document@WebCore@@QAEXXZ to the export file. Unfortunately, I don't have a working copy right now. Please feel free to roll out my patch if that's easier.

It looks like it's only Apple Win Debug (Build). Investigating.

I think it's a problem with the builder. If you look at <http://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/62203/steps/compile-webkit/logs/stdio>, which is the first cycle with my patch, you'll see that it didn't recompile Document.cpp for whatever reason. Those functions are defined in Document.cpp and not used outside WebCore. They shouldn't need to be exported. (Note that the release build also works fine.) We should try a clobber build.

The bot is now healed after another patch touched Platform.h: http://build.webkit.org/waterfall?show=Apple%20Win%20Debug%20(Build) I suspect there's some sort of bad dependency in the Windows build.

It looks like the patch broke fast/frames/seamless/seamless-inherited-origin.html For now, I marked the test as FAIL. Please take a look: https://bugs.webkit.org/show_bug.cgi?id=109391