Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(281)

Issue 1684042: Cleanup Jobs and AncestorChains. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
16 years ago by MikeSamuel
Modified:
15 years, 11 months ago
Reviewers:
Jasvir
CC:
google-caja-discuss_googlegroups.com
Base URL:
http://google-caja.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Gets rid of unneeded complexity in AncestorChains and replaces AncestorChains with ParseTreeNodes as the roots of jobs to make jobs easier to cache. Submitted @4172

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+102 lines, -178 lines) Patch
M src/com/google/caja/opensocial/DefaultGadgetRewriter.java View 2 chunks +1 line, -2 lines 0 comments Download
M src/com/google/caja/parser/AncestorChain.java View 1 chunk +0 lines, -30 lines 0 comments Download
M src/com/google/caja/plugin/BuildServiceImplementation.java View 4 chunks +6 lines, -11 lines 0 comments Download
M src/com/google/caja/plugin/Job.java View 4 chunks +11 lines, -21 lines 0 comments Download
M src/com/google/caja/plugin/PluginCompiler.java View 3 chunks +9 lines, -5 lines 0 comments Download
M src/com/google/caja/plugin/PluginCompilerMain.java View 2 chunks +1 line, -4 lines 0 comments Download
M src/com/google/caja/plugin/stages/CompileHtmlStage.java View 4 chunks +9 lines, -10 lines 0 comments Download
M src/com/google/caja/plugin/stages/ConsolidateCodeStage.java View 3 chunks +2 lines, -4 lines 0 comments Download
M src/com/google/caja/plugin/stages/DebuggingSymbolsStage.java View 3 chunks +6 lines, -9 lines 0 comments Download
M src/com/google/caja/plugin/stages/HtmlToBundleStage.java View 2 chunks +4 lines, -6 lines 0 comments Download
M src/com/google/caja/plugin/stages/HtmlToJsStage.java View 3 chunks +4 lines, -4 lines 0 comments Download
M src/com/google/caja/plugin/stages/InferFilePositionsStage.java View 1 chunk +2 lines, -2 lines 0 comments Download
M src/com/google/caja/plugin/stages/InlineCssImportsStage.java View 1 chunk +1 line, -1 line 0 comments Download
M src/com/google/caja/plugin/stages/JobCache.java View 1 chunk +4 lines, -0 lines 0 comments Download
M src/com/google/caja/plugin/stages/LegacyNamespaceFixupStage.java View 1 chunk +1 line, -1 line 0 comments Download
M src/com/google/caja/plugin/stages/OpenTemplateStage.java View 1 chunk +1 line, -1 line 0 comments Download
M src/com/google/caja/plugin/stages/ResolveUriStage.java View 2 chunks +2 lines, -4 lines 0 comments Download
M src/com/google/caja/plugin/stages/RewriteCssStage.java View 1 chunk +1 line, -1 line 0 comments Download
M src/com/google/caja/plugin/stages/RewriteHtmlStage.java View 3 chunks +3 lines, -5 lines 0 comments Download
M src/com/google/caja/plugin/stages/SanitizeHtmlStage.java View 1 chunk +1 line, -1 line 0 comments Download
M src/com/google/caja/plugin/stages/ValidateCssStage.java View 1 chunk +2 lines, -3 lines 0 comments Download
M src/com/google/caja/plugin/stages/ValidateJavascriptStage.java View 4 chunks +4 lines, -5 lines 0 comments Download
M src/com/google/caja/service/HtmlHandler.java View 2 chunks +1 line, -2 lines 0 comments Download
M tests/com/google/caja/demos/benchmarks/BenchmarkRunner.java View 2 chunks +3 lines, -4 lines 0 comments Download
M tests/com/google/caja/demos/benchmarks/BenchmarkSize.java View 2 chunks +1 line, -2 lines 0 comments Download
M tests/com/google/caja/parser/js/ParserTest.java View 1 chunk +4 lines, -1 line 1 comment Download
M tests/com/google/caja/plugin/HtmlCompiledPluginTest.java View 3 chunks +2 lines, -4 lines 0 comments Download
M tests/com/google/caja/plugin/stages/DebuggingSymbolsStageTest.java View 3 chunks +3 lines, -5 lines 0 comments Download
M tests/com/google/caja/plugin/stages/LegacyNamespaceFixupStageTest.java View 3 chunks +2 lines, -6 lines 0 comments Download
M tests/com/google/caja/plugin/stages/OpenTemplateStageTest.java View 4 chunks +4 lines, -6 lines 0 comments Download
M tests/com/google/caja/plugin/stages/PipelineStageTestCase.java View 3 chunks +5 lines, -13 lines 0 comments Download
M tests/com/google/caja/plugin/stages/RewriteHtmlStageTest.java View 3 chunks +2 lines, -5 lines 0 comments Download

Messages

Total messages: 4
MikeSamuel
16 years ago (2010-06-16 22:06:27 UTC) #1
MikeSamuel
ping
15 years, 12 months ago (2010-06-30 22:22:01 UTC) #2
MikeSamuel
ping
15 years, 11 months ago (2010-07-12 19:18:30 UTC) #3
Jasvir
15 years, 11 months ago (2010-07-12 20:25:10 UTC) #4
LGTM

I am not sure about the value of outlining of next/prev from AncestorChain but
its merely a nitpick.

http://codereview.appspot.com/1684042/diff/1/2
File tests/com/google/caja/parser/js/ParserTest.java (right):

http://codereview.appspot.com/1684042/diff/1/2#newcode648
tests/com/google/caja/parser/js/ParserTest.java:648: int indexInParent =
nChain.parent != null
I am not sure if removing from AncestorChain and outlining this code here is the
right readability trade-off.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b