Bug 52876

Summary:

Chromium DRT should clear page between tests like what Mac DRT does

Product:

WebKit

Reporter:

Xianzhu Wang <wangxianzhu>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED WONTFIX

Severity:

Normal

CC:

ap, dglazkov, eric, phnixwxz, tony

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
patch	none
patch	tony: review-, tony: commit-queue-
new patch that clears page between tests.	tony: review-, tony: commit-queue-
updated patch	none

Xianzhu Wang

Reported 2011-01-21 02:13:18 PST

For now Chromium DRT loads the next test directly after the previous test. Resources used by the previous test that are not GC'ed yet has potential influence on the next test, increasing the chance of flakyness. For example, on an embedded system the number of active GL contexts has a small limit. When running the webgl tests in batch, the GL contexts allocated by the previous test may cause some later test fails to allocate a GL context. I'll upload a patch that adds a command-line option like '--clear-between-tests' to Chromium DRT.

Attachments
patch (3.62 KB, patch) 2011-01-21 02:16 PST, Xianzhu Wang	no flags	Details Formatted Diff Diff
patch (3.64 KB, patch) 2011-01-21 02:20 PST, Xianzhu Wang	tony: review- tony: commit-queue-	Details Formatted Diff Diff
new patch that clears page between tests. (1.17 KB, patch) 2011-01-25 05:39 PST, Xianzhu Wang	tony: review- tony: commit-queue-	Details Formatted Diff Diff
updated patch (2.61 KB, patch) 2011-01-26 19:41 PST, Xianzhu Wang	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Xianzhu Wang

Comment 1 2011-01-21 02:16:51 PST

Created attachment 79713 [details] patch

Eric Seidel (no email)

Comment 2 2011-01-21 02:20:25 PST

I don't see why this should be an option. Either we should do this every time or not at all, or? This is for debugging I guess? Maybe it's useful for tracking down possible bleeding between tests?

Xianzhu Wang

Comment 3 2011-01-21 02:20:29 PST

Created attachment 79714 [details] patch

Eric Seidel (no email)

Comment 4 2011-01-21 02:21:32 PST

If we add this option won't we want to add some sort of support for using it to ORWT/NRWT?

Xianzhu Wang

Comment 5 2011-01-21 03:03:12 PST

(In reply to comment #2) > I don't see why this should be an option. Either we should do this every time or not at all, or? This is for debugging I guess? Maybe it's useful for tracking down possible bleeding between tests? (In reply to comment #4) > If we add this option won't we want to add some sort of support for using it to ORWT/NRWT? The original intent of making it an option was to let specific ports to determine if the option is needed when running DRT. I'm also glad to let it do every time. If this is OK, I'll create a new patch for this.

Tony Chang

Comment 6 2011-01-21 10:05:09 PST

(In reply to comment #0) > For now Chromium DRT loads the next test directly after the previous test. Resources used by the previous test that are not GC'ed yet has potential influence on the next test, increasing the chance of flakyness. > > For example, on an embedded system the number of active GL contexts has a small limit. When running the webgl tests in batch, the GL contexts allocated by the previous test may cause some later test fails to allocate a GL context. Why do the later tests fail? Are they timing out because they can't GC or are the running out of contexts? Do we need to be smarter about running GC when we have a few GL contexts on a low memory device? Is this a problem for the other DRTs (e.g., on Mac)?

Xianzhu Wang

Comment 7 2011-01-21 22:45:51 PST

(In reply to comment #6) > Why do the later tests fail? Are they timing out because they can't GC or are the running out of contexts? Do we need to be smarter about running GC when we have a few GL contexts on a low memory device? > Yes, there might be ways to trigger GC when needed. > Is this a problem for the other DRTs (e.g., on Mac)? I don't know but I guess some flakyness might be caused by the problem, but I can't prove this with my limited resources. Because I make the feature an option, existing automatic tests won't be affected, and we could do some tests with the option enabled. BTW, I saw mac and gtk versions of DRT do load 'about:blank' (without calling GC) between tests.

Tony Chang

Comment 8 2011-01-24 11:32:26 PST

(In reply to comment #7) > BTW, I saw mac and gtk versions of DRT do load 'about:blank' (without calling GC) between tests. We should try to match the mac version of DRT as closely as possible. It sounds like rather than adding a command line flag, we should just load about:blank between tests.

Xianzhu Wang

Comment 9 2011-01-25 05:39:23 PST

Created attachment 80052 [details] new patch that clears page between tests.

Tony Chang

Comment 10 2011-01-25 09:42:25 PST

Comment on attachment 80052 [details] new patch that clears page between tests. It looks like the mac port actual loads the HTML string <html></html> with the URL about:blank. Does it matter? The mac port also immediately calls stopLoading(). Do we need to do that too? The ChangeLog for Mac DRT says this is to delay onload handlers, so it doesn't seem like Mac DRT has the same problem with GL contexts. I suspect there is a real chromium/v8 bug here and this is going to make it harder to fix that bug. However, being more consistent with the Mac DRT is a good thing, so I think this change is worthwhile. In the ChangeLog, please explain why this is being added to the Chromium DRT. r- mainly for ChangeLog.

Xianzhu Wang

Comment 11 2011-01-25 17:45:59 PST

(In reply to comment #10) > (From update of attachment 80052 [details]) > It looks like the mac port actual loads the HTML string <html></html> with the URL about:blank. Does it matter? @Alexey, what's the difference between [mainFrame loadHTMLString:@"<html></html>" baseURL:[NSURL URLWithString:@"about:blank"]] and [mainFrame loadRequest:[NSURLRequest requestWithURL:url]] (where the url is about:blank) ? > The mac port also immediately calls stopLoading(). Do we need to do that too? > Yes, I think so. > The ChangeLog for Mac DRT says this is to delay onload handlers, so it doesn't seem like Mac DRT has the same problem with GL contexts. I suspect there is a real chromium/v8 bug here and this is going to make it harder to fix that bug. However, being more consistent with the Mac DRT is a good thing, so I think this change is worthwhile. > The purpose of this CL has changed. Now the intent is only to keep consistent with Mac DRT. It won't help GL contexts without calling GC. It shouldn't be a v8 bug. The automatic GC triggering mechanism is unable to detect resources (e.g. GL contexts) that is not controlled by it. We must trigger it from the outside of the script engine. I've filed a bug http://code.google.com/p/chromium/issues/detail?id=70736 about that issue in Chromium and will create a CL for it. About clearing between tests, I still think we should do some experiments (which might need the test bots and the --clear-between-tests option helps) to prove if loading 'about:blank' and GC between tests could reduce flakyness. Without GC, some resources allocated in the current page will be destructed when testing later pages which might cause uncertainty. > In the ChangeLog, please explain why this is being added to the Chromium DRT. > OK. > r- mainly for ChangeLog.

Alexey Proskuryakov

Comment 12 2011-01-25 19:22:06 PST

> @Alexey, what's the difference between > [mainFrame loadHTMLString:@"<html></html>" baseURL:[NSURL URLWithString:@"about:blank"]] > and > [mainFrame loadRequest:[NSURLRequest requestWithURL:url]] (where the url is about:blank) ? These are hugely different code paths in WebCore - see the implementation at <http://trac.webkit.org/browser/trunk/Source/WebKit/mac/WebView/WebFrame.mm#L1512>. I don't know what makes the difference in this case.

Xianzhu Wang

Comment 13 2011-01-26 19:41:25 PST

Created attachment 80289 [details] updated patch This patch is only for FYI. I did some comparison tests and loading "about:blank" seemed to make nothing difference. Will continue to investigate if loading blank page and GC could reduce flakyness.

Xianzhu Wang

Comment 14 2012-12-10 13:52:52 PST

Seems not needed any more. Close.

Note You need to log in before you can comment on or make changes to this bug.