Skip to content

Commit

Permalink
reviewed code for memory leak in JavaScript, tuned refresh cycle dete…
Browse files Browse the repository at this point in the history 
…ction (#326)
  • Loading branch information
@ahus1
ahus1 committed Sep 7, 2019
1 parent 76f9d11 commit 82ccf16
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 103 deletions.
86 changes: 63 additions & 23 deletions src/main/java/org/asciidoc/intellij/editor/javafx/JavaFxHtmlPanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,15 @@ protected String compute() {
private String base;

private int lineCount;
private int offset;

private final Path imagesPath;

private VirtualFile parentDirectory;
private VirtualFile saveImageLastDir = null;

private volatile CountDownLatch rendered;
private volatile CountDownLatch rendered = new CountDownLatch(1);
private volatile boolean forceRefresh = false;
private volatile long stamp = 0;

JavaFxHtmlPanel(Document document, Path imagesPath) {

Expand Down Expand Up @@ -420,8 +421,15 @@ public JComponent getComponent() {
}

@Override
public void setHtml(@NotNull String htmlParam) {
public synchronized void setHtml(@NotNull String htmlParam) {
rendered = new CountDownLatch(1);
stamp += 1;
if (stamp > 10000) {
// force a refresh to avoid memory leaks
forceRefresh = true;
stamp = 0;
}
long iterationStamp = stamp;
runInPlatformWhenAvailable(() -> {
String html = htmlParam;
if (isDarcula()) {
Expand All @@ -431,17 +439,20 @@ public void setHtml(@NotNull String htmlParam) {
}
boolean result = false;
final AsciiDocApplicationSettings settings = AsciiDocApplicationSettings.getInstance();
if (settings.getAsciiDocPreviewSettings().isInplacePreviewRefresh() && html.contains("id=\"content\"")) {
if (!forceRefresh && settings.getAsciiDocPreviewSettings().isInplacePreviewRefresh() && html.contains("id=\"content\"")) {
final String htmlToReplace = StringEscapeUtils.escapeEcmaScript(prepareHtml(html));
// try to replace the HTML contents using JavaScript to avoid flickering MathML
try {
boolean ml = false; // set to "true" to test for memory leaks in HTML/JavaScript
result = (Boolean) JavaFxHtmlPanel.this.getWebViewGuaranteed().getEngine().executeScript(
(ml ? "var x = 0; " : "") +
"function finish() {" +
"if ('__IntelliJTools' in window) {" +
"__IntelliJTools.processLinks && __IntelliJTools.processLinks();" +
"__IntelliJTools.pickSourceLine && __IntelliJTools.pickSourceLine(" + lineCount + ", " + offset + ");" +
"__IntelliJTools.pickSourceLine && __IntelliJTools.pickSourceLine(" + lineCount + ");" +
(ml ? "window.setTimeout(function(){ updateContent(); }, 10); " : "") +
"}" +
"window.JavaPanelBridge && window.JavaPanelBridge.rendered();" +
"window.JavaPanelBridge && window.JavaPanelBridge.rendered(" + iterationStamp + ");" +
"}" +
"function updateContent() { var elem = document.getElementById('content'); if (elem && elem.parentNode) { " +
"var div = document.createElement('div');" +
Expand All @@ -452,14 +463,28 @@ public void setHtml(@NotNull String htmlParam) {
" errortext.textContent = ''; " +
" errorformula.textContent = ''; " +
"} " +
(ml ? "x = x + 1; " : "") +
(ml ? "errortext.textContent = 'count:' + x; " : "") +
"div.style.cssText = 'display: none'; " +
// need to add the element to the DOM as MathJAX will use document.getElementById in some places
"elem.appendChild(div); " +
// use MathJax to set the formulas in advance if formulas are present - this takes ~100ms
// re-evaluate the content element as it might have been replaced by a concurrent rendering
"if ('MathJax' in window && MathJax.Hub.getAllJax().length > 0) { MathJax.Hub.Typeset(div.firstChild, function() { var elem2 = document.getElementById('content'); elem2.parentNode.replaceChild(div.firstChild, elem2); finish(); }); } " +
"if ('MathJax' in window && MathJax.Hub.getAllJax().length > 0) { " +
"MathJax.Hub.Typeset(div.firstChild, function() { " +
"var elem2 = document.getElementById('content'); " +
"__IntelliJTools.clearLinks && __IntelliJTools.clearLinks();" +
"__IntelliJTools.clearSourceLine && __IntelliJTools.clearSourceLine();" +
"elem2.parentNode.replaceChild(div.firstChild, elem2); " +
"finish(); }); } " +
// if no math was present before, replace contents, and do the MathJax typesetting afterwards in case Math has been added
"else { elem.parentNode.replaceChild(div.firstChild, elem); MathJax.Hub.Typeset(div.firstChild); finish(); } " +
"else { " +
"__IntelliJTools.clearLinks && __IntelliJTools.clearLinks();" +
"__IntelliJTools.clearSourceLine && __IntelliJTools.clearSourceLine();" +
"elem.parentNode.replaceChild(div.firstChild, elem); " +
"MathJax.Hub.Typeset(div.firstChild); " +
"finish(); " +
"} " +
"return true; } else { return false; }}; updateContent();"
);
} catch (RuntimeException e) {
Expand All @@ -469,18 +494,23 @@ public void setHtml(@NotNull String htmlParam) {
}
// if not successful using JavaScript (like on first rendering attempt), set full content
if (!result) {
html = "<html><head></head><body><div style='position:fixed;top:0;left:0;background-color:#eeeeee;color:red;z-index:99;'><div id='mathjaxerrortext'></div><pre style='color:red' id='mathjaxerrorformula'></pre></div>" + html + "</body>";
forceRefresh = false;
html = "<html><head></head><body><div style='position:fixed;top:0;left:0;background-color:#eeeeee;color:red;z-index:99;'><div id='mathjaxerrortext'></div><pre style='color:red' id='mathjaxerrorformula'></pre></div>"
+ html
+ "<script>window.iterationStamp=" + iterationStamp + " </script></body>";
final String htmlToRender = prepareHtml(html);
JavaFxHtmlPanel.this.getWebViewGuaranteed().getEngine().loadContent(htmlToRender);
rendered.countDown();
}
});
try {
// slow down the rendering of the next version of the preview until the rendering if the current version is complete
// this prevents us building up a queue that would lead to a lagging preview
rendered.await(1, TimeUnit.SECONDS);
if (htmlParam.length() > 0 && !rendered.await(3, TimeUnit.SECONDS)) {
log.warn("rendering didn't complete in time, might be slow or broken");
forceRefresh = true;
}
} catch (InterruptedException e) {
log.warn("rendering didn't complete in time, might be slow or broken");
log.warn("interrupted while waiting for refresh to complete");
}
}

Expand Down Expand Up @@ -626,8 +656,10 @@ public void openLink(@NotNull String link) {
}
}

public void rendered() {
rendered.countDown();
public void rendered(long iterationStamp) {
if (stamp == iterationStamp) {
rendered.countDown();
}
}

private boolean openInEditor(@NotNull URI uri) {
Expand Down Expand Up @@ -710,15 +742,23 @@ private class BridgeSettingListener implements ChangeListener<State> {
@Override
public void changed(ObservableValue<? extends State> observable, State oldValue, State newValue) {
if (newValue == State.SUCCEEDED) {
JSObject win
= (JSObject) getWebViewGuaranteed().getEngine().executeScript("window");
win.setMember("JavaPanelBridge", bridge);
JavaFxHtmlPanel.this.getWebViewGuaranteed().getEngine().executeScript(
"if ('__IntelliJTools' in window) {" +
"__IntelliJTools.processLinks && __IntelliJTools.processLinks();" +
"__IntelliJTools.pickSourceLine && __IntelliJTools.pickSourceLine(" + lineCount + ", " + offset + ");" +
"}"
);
try {
JSObject win
= (JSObject) getWebViewGuaranteed().getEngine().executeScript("window");
win.setMember("JavaPanelBridge", bridge);
JavaFxHtmlPanel.this.getWebViewGuaranteed().getEngine().executeScript(
"if ('__IntelliJTools' in window) {" +
"__IntelliJTools.processLinks && __IntelliJTools.processLinks();" +
"__IntelliJTools.pickSourceLine && __IntelliJTools.pickSourceLine(" + lineCount + ");" +
"}" +
"JavaPanelBridge.rendered(window.iterationStamp);"
);
} catch (RuntimeException e) {
// might happen when rendered output is not valid HTML due to passtrough content
log.warn("unable to initialize page JavaScript", e);
forceRefresh = true;
rendered.countDown();
}
}
}
}
Expand Down
95 changes: 48 additions & 47 deletions src/main/resources/org/asciidoc/intellij/editor/javafx/pickSourceLine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,84 +2,85 @@ if (window.__IntelliJTools === undefined) {
window.__IntelliJTools = {}
}

window.__IntelliJTools.pickSourceLine = (function () {

var offset = 0;

var getLine = function (node) {
if (!node || !('className' in node)) {
return null
}
var classes = node.className.split(' ');
window.__IntelliJTools.getLine = function (node) {
if (!node || !('className' in node)) {
return null
}
var classes = node.className.split(' ');

for (var i = 0; i < classes.length; i++) {
var className = classes[i]
if (className.match(/^data-line-stdin-/)) {
return Number(className.substr("data-line-stdin-".length));
}
for (var i = 0; i < classes.length; i++) {
var className = classes[i]
if (className.match(/^data-line-stdin-/)) {
return Number(className.substr("data-line-stdin-".length));
}

return null
}

var lineCount;
return null
}

window.__IntelliJTools.lineCount = 0;

function calculateOffset(element) {
var offset = 0
while(element != null) {
offset += element.offsetTop
element = element.offsetParent
}
return offset
window.__IntelliJTools.calculateOffset = function (element) {
var offset = 0
while (element != null) {
offset += element.offsetTop
element = element.offsetParent
}
return offset
}

window.__IntelliJTools.scrollEditorToLine = function (event) {
var sourceLine = getLine(this)
window.__IntelliJTools.scrollEditorToLine = function (event) {
try {
var sourceLine = window.__IntelliJTools.getLine(this)

var blocks = document.getElementsByClassName('has-source-line');
var startY;
var startLine;
var endY;
var endLine = lineCount;
var endLine = window.__IntelliJTools.lineCount;

for (var i = 0; i < blocks.length; i++) {
var block = blocks[i]
var lineOfBlock = getLine(block);
var lineOfBlock = window.__IntelliJTools.getLine(block);
if (lineOfBlock <= sourceLine) {
startY = calculateOffset(block)
startY = window.__IntelliJTools.calculateOffset(block)
startLine = lineOfBlock
// there might be no further block, therefore assume that the end is at the end of this block
endY = startY + block.offsetHeight
}
else if (lineOfBlock > sourceLine) {
endY = calculateOffset(block)
endLine = lineOfBlock -1;
} else if (lineOfBlock > sourceLine) {
endY = window.__IntelliJTools.calculateOffset(block)
endLine = lineOfBlock - 1;
break
}
}

var editorLine = startLine + (event.clientY + window.scrollY - startY) * (endLine - startLine) / (endY - startY)

window.JavaPanelBridge.scrollEditorToLine(editorLine - offset)
window.JavaPanelBridge.scrollEditorToLine(editorLine)
event.stopPropagation()
} catch (e) {
window.JavaPanelBridge.log("can't pick source line", JSON.stringify(e));
}
}

var initializeContent = function (lc, off) {
window.__IntelliJTools.pickSourceLine = function (lc) {

offset = off
// the sourcelines will be as CSS class elements that also have class has-source-line
var blocks = document.getElementsByClassName('has-source-line');

// the sourcelines will be as CSS class elements that also have class has-source-line
var blocks = document.getElementsByClassName('has-source-line');
for (var i = 0; i < blocks.length; i++) {
blocks[i].addEventListener('click', window.__IntelliJTools.scrollEditorToLine);
}

for (var i = 0; i < blocks.length; i++) {
blocks[i].onclick = window.__IntelliJTools.scrollEditorToLine;
}
window.__IntelliJTools.lineCount = lc;

lineCount = lc + off;
}

}
window.__IntelliJTools.clearSourceLine = function () {

return initializeContent
// the sourcelines will be as CSS class elements that also have class has-source-line
var blocks = document.getElementsByClassName('has-source-line');

})()
for (var i = 0; i < blocks.length; i++) {
blocks[i].removeEventListener('click', window.__IntelliJTools.scrollEditorToLine);
}

}
66 changes: 33 additions & 33 deletions src/main/resources/org/asciidoc/intellij/editor/javafx/processLinks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,45 +2,45 @@ if (window.__IntelliJTools === undefined) {
window.__IntelliJTools = {}
}

window.__IntelliJTools.processLinks = (function () {
var openLink = function (href) {
window.JavaPanelBridge.openLink(href);
}

window.__IntelliJTools.processClick = function (event) {
// stop propagation to prevent triggering scrolling to source line
event.stopPropagation()

if (!this.href) {
return false;
}

if (this.href[0] == '#') {
var elementId = this.href.substring(1)
var elementById = document.getElementById(elementId);
if (elementById) {
elementById.scrollIntoView();
}
}
else {
openLink(this.href);
}
window.__IntelliJTools.processClick = function (event) {
// stop propagation to prevent triggering scrolling to source line
event.stopPropagation()

if (!this.href) {
return false;
}

var processLinks = function () {
var links = document.getElementsByTagName("a");
// window.JavaPanelBridge.log(links.length)
for (var i = 0; i < links.length; ++i) {
var link = links[i];

link.onclick = __IntelliJTools.processClick
// window.JavaPanelBridge.log(link + ' ' + link.onclick)
if (this.href[0] == '#') {
var elementId = this.href.substring(1)
var elementById = document.getElementById(elementId);
if (elementById) {
elementById.scrollIntoView();
}
} else {
window.JavaPanelBridge.openLink(this.href);
}

return processLinks;
return false;
}

window.__IntelliJTools.processLinks = function () {
var links = document.getElementsByTagName("a");
// window.JavaPanelBridge.log(links.length)
for (var i = 0; i < links.length; ++i) {
var link = links[i];

})()
link.addEventListener('click', window.__IntelliJTools.processClick);
// window.JavaPanelBridge.log(link + ' ' + link.onclick)
}
}

window.__IntelliJTools.clearLinks = function () {
var links = document.getElementsByTagName("a");
// window.JavaPanelBridge.log(links.length)
for (var i = 0; i < links.length; ++i) {
var link = links[i];

link.removeEventListener('click', __IntelliJTools.processClick);
// window.JavaPanelBridge.log(link + ' ' + link.onclick)
}
}

0 comments on commit 82ccf16

Please sign in to comment.