Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upReddit.com doesn't load completely within browser.html #11223
Comments
|
Just to confirm, the loaders we are talking about in |
|
This script is added:
… and never removed from the loader list. |
|
So we never get the headers for this script. So I guess it's a network issue. |
|
I'm wondering if I'm hitting the ssl issue of #11008 I'm not getting headers (at least, |
|
This seems to only happen on slower machines, or with fewer CPU cores: on a 2012 MBP with 4 (physical) cores, I can't reproduce at all, whereas on a 2015 Macbook with 2 cores, it happens reliably. |
|
@tschneidereit have you tried rr's random scheduler? @nox This might be a good bug to take. |
|
Will look into that. |
|
CPU usage hits ~100% while that happens. I'll attempt to profile, that might help us understand if we get stuck somewhere. |
|
Also - depending on time, reddit doesn't serve the same ads, so it might not happen all the time. At the moment, it sill happens, but with a different URL. |
|
Result of the profile after the loadend got blocked: http://people.mozilla.org/~prouget/bugs/issue11223.svg (csv version: http://people.mozilla.org/~prouget/bugs/issue11223.csv). |
|
@till / @Gozala can you guys check (or tell me how to check) why we have JS code running while the iframe is not sending event anymore? PS: I'm disabling the progressbar like this, is that enough? diff --git a/src/browser/web-view.js b/src/browser/web-view.js
index a20291d..5e400d0 100644
--- a/src/browser/web-view.js
+++ b/src/browser/web-view.js
@@ -810,7 +810,7 @@ export const view =
)
]
)
- , Progress.view(model.progress, address)
+ // , Progress.view(model.progress, address)
, html.div
( { className: 'webview-tab-icon'
, style: |
|
I think it's also necessary to remove |
|
Commenting out Using So basically, when CPU gets busy, a script might never be executed/downloaded (not sure yet) and blocks the document loader. That also explains why we can not reproduce this issue on a Macbook Pro (faster CPU). @nox, is there anything I can do to help profiling/debugging this? |
|
@paulrouget Could we somehow list the blocking loads stored in |
|
Do you think we can reproduce it on a better computer by using |
|
Is the HTML sample in browserhtml/browserhtml#1037 still relevant? Seems like it happens 100% of the time when I try locally with it. |
|
Oh, not 100% of the time, but I can reproduce it on my fancy laptop at least. |
|
Actually I had just missed the |
Not sure to understand the question. You want to
I haven't tried, but I guess if you nice-up the the process or somehow manage to limit the number of available cores, that might help. To reproduce, something "heavy" needs to happen on the side. browserhtml happens to have an expensive |
Just tried. Very expensive |
|
@nox Mind if I assign you here? |
|
@pcwalton Done. |
|
So for the script mentioned in my 2nd comment, headers are being downloaded. I can tell because the script go through this: servo/components/net/resource_thread.rs Line 145 in dea6109 start_sending_opt).
But I think the listener is not called. I added a servo/components/net_traits/lib.rs Line 155 in dea6109 ResponseAction::process) and I don't see anything for this specific script.
2 questions:
|
|
Ignore previous comment. I'm getting |
|
The script is being downloaded via
I know nothing about IPC, but I guess that's not good. |
|
Doing so leads me to https://www.redditstatic.com/ad-dependencies.5O7sMdAReBw.js but I don't know if I can trust anything about the line and column information, since Servo routinely fudges those. |
|
That file seems to be the only interesting one that adds a listener for |
|
My current theory is that we end up in an endless loop in JS, since we never return from JS::Interpret. |
|
Here's a backtrace of the thread that doesn't return from the JS interpreter after killing the process: |
|
I did it a second time after waiting for a lot longer and saw the same backtrace. We appear to be stuck waiting on a layout mutex. |
|
Meanwhile, the layout thread for that page doesn't appear to be doing anything interesting: |
|
Interestingly, despite the script thread having returned from the |
|
The layout thread does have |
|
The reflow doesn't happen because |
|
Also, while trying to find a test for this (before knowing what @jdm said), I found out we deadlock in a very simple example: <!doctype html>
<script>
var frame;
setTimeout(function f() {
if (frame)
frame.parentNode.removeChild(frame);
frame = document.createElement('iframe');
frame.src = "frame.html";
frame.style.display = "none";
document.body.appendChild(frame);
setTimeout(f, 150);
}, 150)
</script>(We never manage to remove the first frame, which is worrying) |
|
@jdm: So it seems to reproduce the original error we need a chunked response, right? |
|
Yeah, I assume there's a timing issue where small responses end up getting parsed too quickly or something? I've been having trouble figuring out how the window size messages would end up being delayed, though. |
|
So the only moment when there can be no window_size is while there's an Could it be the case that while the page is loading, a parent script is On Mon, May 30, 2016 at 08:34:29PM -0700, Josh Matthews wrote:
|
|
Does it even make sense to do queries for offsets and whatnot when there is no |
|
So I was able to reproduce it consistently: Take the following <!doctype html>
<title></title>
<iframe src="http://localhost:5000"></iframe>
<div>test</div>and put up a node server with this: const http = require('http');
const PORT = 5000;
const server = http.createServer((_request, res) => {
res.writeHead(200, {
'Content-Type': 'text/html'
});
res.write("<!doctype html><script>console.log(document.documentElement.offsetHeight)</script>");
setTimeout(() => {
res.write("<div>Test</div>");
res.end();
}, 2000)
});
server.listen(PORT, () => console.log("Server listening on: http://localhost:%s", PORT));if you run |
…to the viewport size not yet present. This fixes servo#11223. Note that this didn't happen in the root pipeline because we explicitly set the window size in that case.
script: Ensure we don't ignore reflows for queries that bail out due to the viewport size not yet present <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #11223 <!-- Either: --> - [ ] There are tests for these changes. <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> This fixes #11223. Note that this didn't happen in the root pipeline because we explicitly set the window size in that case. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11545) <!-- Reviewable:end -->
…to the viewport size not yet present. This fixes servo#11223. Note that this didn't happen in the root pipeline because we explicitly set the window size in that case.
script: Ensure we don't ignore reflows for queries that bail out due to the viewport size not yet present <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #11223 <!-- Either: --> - [ ] There are tests for these changes. <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> This fixes #11223. Note that this didn't happen in the root pipeline because we explicitly set the window size in that case. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11545) <!-- Reviewable:end -->
|
Thank you all for figuring out what was going on! It now works perfectly. |
…to the viewport size not yet present. This fixes servo#11223. Note that this didn't happen in the root pipeline because we explicitly set the window size in that case.
Duplicate of browserhtml/browserhtml#1037 as this is apparently a servo bug.
Often the mozbrowser iframe doesn't get the
loadendevent when loading reddit.com.It appears to happen only with browserhtml.
This is what I managed to figure out so far:
finish_load, for thereddit.compipeline,DocumentLoadsCompleteis properly sent. Which means thatloader.is_blocked()returnfalse.script_thread.rs, inhandle_loads_complete,doc.loader().is_blocked()returnstrue, preventing finishing the load.So I guess we need to figure out why sometimes the loader gets blocked.