Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(147)

Issue 934083002: Remove the concept of pendingScripts from HTMLScriptRunner (Closed)

Created:
5 years, 10 months ago by eseidel
Modified:
5 years, 10 months ago
Reviewers:
abarth-chromium
CC:
abarth-chromium, esprehn, mojo-reviews_chromium.org, ojan, qsr+mojo_chromium.org
Base URL:
git@github.com:domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Remove the concept of pendingScripts from HTMLScriptRunner We shouldn't need pendingScripts in ScriptRunner since we should never be trying to run scripts when we're not ready to run them. However this wasn't completely true in the case of imports there was code to have us break before any start tag after an <import> was seen, but it was subtly wrong in that it it would include the start-tag it was trying to break before in the chunk it sent to the main thread. This didn't run out to be the problem I was facing, but I fixed it anyway. The problem which was actually preventing me from removing pendingScripts was adding a check inside didRecieveParsedChunk... to check if imports were pending and add the chunk to the list of pending chunks. I also renamed m_speculations to m_pendingChunks since these chunks are never speculative anymore. We can't test the off-by-one import-breaking code with our current system, but it would be trivial to test with a self-closing custom element if/when we ever add custom elements back to the system. R=abarth@chromium.org BUG= Committed: https://chromium.googlesource.com/external/mojo/+/699c4cc34b01ea80e125f166374be508d5de3b2c

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -55 lines) Patch
M sky/engine/core/dom/Document.h View 1 chunk +0 lines, -2 lines 0 comments Download
M sky/engine/core/html/parser/BackgroundHTMLParser.h View 1 chunk +8 lines, -1 line 0 comments Download
M sky/engine/core/html/parser/BackgroundHTMLParser.cpp View 2 chunks +22 lines, -8 lines 0 comments Download
M sky/engine/core/html/parser/HTMLDocumentParser.h View 2 chunks +2 lines, -2 lines 0 comments Download
M sky/engine/core/html/parser/HTMLDocumentParser.cpp View 8 chunks +19 lines, -20 lines 0 comments Download
M sky/engine/core/html/parser/HTMLScriptRunner.h View 1 chunk +0 lines, -4 lines 0 comments Download
M sky/engine/core/html/parser/HTMLScriptRunner.cpp View 1 chunk +1 line, -18 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
abarth-chromium
lgtm
5 years, 10 months ago (2015-02-17 23:58:17 UTC) #1
eseidel
5 years, 10 months ago (2015-02-18 00:13:34 UTC) #2
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
699c4cc34b01ea80e125f166374be508d5de3b2c (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698