Prepare blink to unify definitions of load completion

Prepare blink to unify definitions of load completion

Currently, FrameLoader.cpp contains a function called isDocumentDoneLoading,
which has a complicated set of rules to determine when to tell chromium that
loading is complete. These rules only really exist for the sake of layout
tests, which can't always just define load event completion as the end of
the test. A set of rules have been added to the layout tests harness behind
a build flag, so add a flagged version of load completion in blink that is
strongly mapped to load event completion.

Lots of tests need subtle changes. Most of these are forced layouts, waiting for RAF to complete, simple waitUntilDone/notifyDone pairs, and adding missing document.close() calls.

BUG=426520
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184389

[Nate Chapin, 2014-09-15 23:39:32]

japhet@chromium.org changed reviewers:
+ abarth@chromium.org

[Nate Chapin, 2014-09-15 23:39:35]

Sorry for the size of this...

https://codereview.chromium.org/561813003/diff/100001/Source/bindings/core/v8...
File Source/bindings/core/v8/ScriptController.cpp (right):

https://codereview.chromium.org/561813003/diff/100001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptController.cpp:540:
m_frame->loader().progress().progressStarted();
Apparently we've never marked the start of a navigation for a javascript url.
Huh.

https://codereview.chromium.org/561813003/diff/100001/Source/core/dom/Documen...
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/561813003/diff/100001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:4432: setParsing(true);
XSLT is processed after the main xml document has completely parsed, but is
conceptually part of document parsing, so set parsing state accordingly.

https://codereview.chromium.org/561813003/diff/100001/Source/core/dom/Process...
File Source/core/dom/ProcessingInstruction.cpp (right):

https://codereview.chromium.org/561813003/diff/100001/Source/core/dom/Process...
Source/core/dom/ProcessingInstruction.cpp:222:
OwnPtr<IncrementLoadEventDelayCount> delay =
IncrementLoadEventDelayCount::create(document());
XSL stylesheet parsing should block the load event.

https://codereview.chromium.org/561813003/diff/100001/Source/core/fetch/Resou...
File Source/core/fetch/Resource.h (right):

https://codereview.chromium.org/561813003/diff/100001/Source/core/fetch/Resou...
Source/core/fetch/Resource.h:179: void setLoadFinishTime(double finishTime) {
m_loadFinishTime = finishTime; }
Split these out to guarantee we set timing information before test completion
(hopefully this can eventually get re-simplified).

https://codereview.chromium.org/561813003/diff/100001/Source/core/fetch/Resou...
File Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/561813003/diff/100001/Source/core/fetch/Resou...
Source/core/fetch/ResourceFetcher.cpp:1249: if (resource &&
resource->response().isHTTP() && resource->response().httpStatusCode() < 400 &&
document()) {
dispatchDidFinishLoading might end a test, so ensure we set resource timing
information before then. Also, we only want to set resource timing information
if the load is successful, so didFinishLoading() makes more sense than
didLoadResource() anyway.

https://codereview.chromium.org/561813003/diff/100001/Source/core/loader/Fram...
File Source/core/loader/FrameFetchContext.cpp (right):

https://codereview.chromium.org/561813003/diff/100001/Source/core/loader/Fram...
Source/core/loader/FrameFetchContext.cpp:194:
m_frame->loader().client()->dispatchDidFinishLoading(loader, identifier);
We need to report resource load completion for both success and failure for
layout tests. This callback isn't used for much else.

https://codereview.chromium.org/561813003/diff/100001/Source/core/loader/Fram...
File Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/561813003/diff/100001/Source/core/loader/Fram...
Source/core/loader/FrameLoader.cpp:234: m_progressTracker->progressStarted();
Similar to a javascript url, an explicit open() in an initial empty document
should count as a start.

https://codereview.chromium.org/561813003/diff/100001/Source/core/loader/Fram...
Source/core/loader/FrameLoader.cpp:1308: m_state = FrameStateComplete;
This isn't necessary in the real world at the moment (and may not be necessary
even in an OOPI world). It only really matters if
shouldContinueForNavigationPolicy() asks us to ignore the first real load in an
iframe, which is covered by a layout test.

https://codereview.chromium.org/561813003/diff/100001/Source/core/loader/Navi...
File Source/core/loader/NavigationScheduler.cpp (right):

https://codereview.chromium.org/561813003/diff/100001/Source/core/loader/Navi...
Source/core/loader/NavigationScheduler.cpp:156: : ScheduledURLNavigation(0.0,
originDocument, url, referrer, lockBackForwardList, !protocolIsJavaScript(url))
{ }
A pending navigation to a JS url shouldn't stop html parsing or prevent the load
event. This boolean gets mapped to NavigationScheduler::locationChangePending().

[abarth-chromium, 2014-09-23 20:47:45]

This CL is intimidating to me...  Maybe we can find someone else to review it?

[dcheng, 2014-09-25 17:16:06]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2014-10-20 18:35:13]

Just an initial set of comments, since I'm not sure I entirely grok this yet.

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/editing/exe...
File LayoutTests/editing/execCommand/window-open-insert-list-crash.html (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/editing/exe...
LayoutTests/editing/execCommand/window-open-insert-list-crash.html:25: iframe =
null;
Why is it important that this code only runs once now?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/fast/dom/Im...
File LayoutTests/fast/dom/ImageDocument-image-deletion-expected.txt (left):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/fast/dom/Im...
LayoutTests/fast/dom/ImageDocument-image-deletion-expected.txt:1: Test for
http://bugs.webkit.org/show_bug.cgi?id=16097 Safari crashes during load of
LexisNexis search results.
Why did we delete this test?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/fast/dom/cs...
File LayoutTests/fast/dom/css-insert-import-rule.html (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/fast/dom/cs...
LayoutTests/fast/dom/css-insert-import-rule.html:11:
document.styleSheets[0].insertRule('@import
url("resources/css-insert-import-rule.css");', 0);
Is this just to simplify the test, or was this driven by a correctness issue?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
File LayoutTests/http/tests/incremental/doc-write-before-end.pl (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
LayoutTests/http/tests/incremental/doc-write-before-end.pl:15: print "<img
src='404.gif' onerror='document.write(\"PASS\"); document.close()'>";
Why is the call to document.close() important here? I'm not familiar with the
rules for when it's needed/used.

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
File LayoutTests/http/tests/multipart/invalid-image-data.html (left):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
LayoutTests/http/tests/multipart/invalid-image-data.html:1: <html>
Why is it safe to delete this test?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
File LayoutTests/http/tests/security/cross-frame-access-call.html (left):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
LayoutTests/http/tests/security/cross-frame-access-call.html:62: // Work around
DRT bug that causes subsequent tests to fail.
o.O

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
File LayoutTests/http/tests/xmlhttprequest/send-on-abort.html (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
LayoutTests/http/tests/xmlhttprequest/send-on-abort.html:27: location.href =
"data:text/html,PASS: No assertion failure.<script>if (window.testRunner)
testRunner.notifyDone()</scr" + "ipt>";
What is this significance of the ordering change here. I'm guessing if you call
notifyDone() first the trailing text never goes to layout now? Will this have a
web visible effect?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/svg/custom/...
File LayoutTests/svg/custom/resources/linkingCircle-f.svg (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/svg/custom/...
LayoutTests/svg/custom/resources/linkingCircle-f.svg:7: <svg version="1.1"
xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
id="linkingCircle-f.svg" width="480" height="360" onload="setTimeout(report,
0);">
How come we need to defer this to a setTimeout() now?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/webaudio/sc...
File LayoutTests/webaudio/scriptprocessornode-rewrap.html (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/webaudio/sc...
LayoutTests/webaudio/scriptprocessornode-rewrap.html:18:
document.body.appendChild(i);
This test looks really bizarre. Is this originally a test case generated by
clusterfuzz? What's the purpose of the iframe here?

https://codereview.chromium.org/561813003/diff/120001/Source/bindings/core/v8...
File Source/bindings/core/v8/ScriptController.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptController.cpp:547:
m_frame->loader().progress().progressStarted();
What's the significance to calling this now for JS urls. Don't JS urls load
synchronously anyway, so the progress is always 100% by the time other content
runs? I guess this is an internal fix for consistency?

https://codereview.chromium.org/561813003/diff/120001/Source/core/dom/Documen...
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:2499: suppressLoadEvent();
If I'm reading this correctly, this means we won't fire a load event if we get a
navigation before the document is closed?

https://codereview.chromium.org/561813003/diff/120001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:2499: suppressLoadEvent();
If I'm reading this correctly, this means we won't fire a load event if we get a
navigation before the document is closed?

https://codereview.chromium.org/561813003/diff/120001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:4457: setParsing(true);
It would be kind of nice to use some sort of scoped here (like TemporaryChange,
but that only works with variables).

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
File Source/core/fetch/ResourceLoader.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
Source/core/fetch/ResourceLoader.cpp:475:
m_resource->setLoadFinishTime(finishTime);
What is the significance to splitting this out of finish(). Is the effect web
visible?

https://codereview.chromium.org/561813003/diff/120001/Source/core/loader/Fram...
File Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/core/loader/Fram...
Source/core/loader/FrameLoader.cpp:1017: #if
defined(ENABLE_LOAD_COMPLETION_HACKS)
o.O

[dcheng, 2014-10-20 18:35:14]

Just an initial set of comments, since I'm not sure I entirely grok this yet.

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/editing/exe...
File LayoutTests/editing/execCommand/window-open-insert-list-crash.html (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/editing/exe...
LayoutTests/editing/execCommand/window-open-insert-list-crash.html:25: iframe =
null;
Why is it important that this code only runs once now?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/fast/dom/Im...
File LayoutTests/fast/dom/ImageDocument-image-deletion-expected.txt (left):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/fast/dom/Im...
LayoutTests/fast/dom/ImageDocument-image-deletion-expected.txt:1: Test for
http://bugs.webkit.org/show_bug.cgi?id=16097 Safari crashes during load of
LexisNexis search results.
Why did we delete this test?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/fast/dom/cs...
File LayoutTests/fast/dom/css-insert-import-rule.html (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/fast/dom/cs...
LayoutTests/fast/dom/css-insert-import-rule.html:11:
document.styleSheets[0].insertRule('@import
url("resources/css-insert-import-rule.css");', 0);
Is this just to simplify the test, or was this driven by a correctness issue?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
File LayoutTests/http/tests/incremental/doc-write-before-end.pl (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
LayoutTests/http/tests/incremental/doc-write-before-end.pl:15: print "<img
src='404.gif' onerror='document.write(\"PASS\"); document.close()'>";
Why is the call to document.close() important here? I'm not familiar with the
rules for when it's needed/used.

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
File LayoutTests/http/tests/multipart/invalid-image-data.html (left):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
LayoutTests/http/tests/multipart/invalid-image-data.html:1: <html>
Why is it safe to delete this test?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
File LayoutTests/http/tests/security/cross-frame-access-call.html (left):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
LayoutTests/http/tests/security/cross-frame-access-call.html:62: // Work around
DRT bug that causes subsequent tests to fail.
o.O

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
File LayoutTests/http/tests/xmlhttprequest/send-on-abort.html (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/http/tests/...
LayoutTests/http/tests/xmlhttprequest/send-on-abort.html:27: location.href =
"data:text/html,PASS: No assertion failure.<script>if (window.testRunner)
testRunner.notifyDone()</scr" + "ipt>";
What is this significance of the ordering change here. I'm guessing if you call
notifyDone() first the trailing text never goes to layout now? Will this have a
web visible effect?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/svg/custom/...
File LayoutTests/svg/custom/resources/linkingCircle-f.svg (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/svg/custom/...
LayoutTests/svg/custom/resources/linkingCircle-f.svg:7: <svg version="1.1"
xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"
id="linkingCircle-f.svg" width="480" height="360" onload="setTimeout(report,
0);">
How come we need to defer this to a setTimeout() now?

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/webaudio/sc...
File LayoutTests/webaudio/scriptprocessornode-rewrap.html (right):

https://codereview.chromium.org/561813003/diff/120001/LayoutTests/webaudio/sc...
LayoutTests/webaudio/scriptprocessornode-rewrap.html:18:
document.body.appendChild(i);
This test looks really bizarre. Is this originally a test case generated by
clusterfuzz? What's the purpose of the iframe here?

https://codereview.chromium.org/561813003/diff/120001/Source/bindings/core/v8...
File Source/bindings/core/v8/ScriptController.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/bindings/core/v8...
Source/bindings/core/v8/ScriptController.cpp:547:
m_frame->loader().progress().progressStarted();
What's the significance to calling this now for JS urls. Don't JS urls load
synchronously anyway, so the progress is always 100% by the time other content
runs? I guess this is an internal fix for consistency?

https://codereview.chromium.org/561813003/diff/120001/Source/core/dom/Documen...
File Source/core/dom/Document.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:2499: suppressLoadEvent();
If I'm reading this correctly, this means we won't fire a load event if we get a
navigation before the document is closed?

https://codereview.chromium.org/561813003/diff/120001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:2499: suppressLoadEvent();
If I'm reading this correctly, this means we won't fire a load event if we get a
navigation before the document is closed?

https://codereview.chromium.org/561813003/diff/120001/Source/core/dom/Documen...
Source/core/dom/Document.cpp:4457: setParsing(true);
It would be kind of nice to use some sort of scoped here (like TemporaryChange,
but that only works with variables).

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
File Source/core/fetch/ResourceLoader.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
Source/core/fetch/ResourceLoader.cpp:475:
m_resource->setLoadFinishTime(finishTime);
What is the significance to splitting this out of finish(). Is the effect web
visible?

https://codereview.chromium.org/561813003/diff/120001/Source/core/loader/Fram...
File Source/core/loader/FrameLoader.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/core/loader/Fram...
Source/core/loader/FrameLoader.cpp:1017: #if
defined(ENABLE_LOAD_COMPLETION_HACKS)
o.O

[dcheng, 2014-10-20 19:28:05]

Just two cleanup thoughts from our discussion. I think I need to dive into one
bit of code a bit more, but overall, this looks reasonable.

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
File Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
Source/core/fetch/ResourceFetcher.cpp:1155: void
ResourceFetcher::didLoadResource(Resource* resource)
This can be const now, right?

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
File Source/core/fetch/ResourceLoader.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
Source/core/fetch/ResourceLoader.cpp:237: m_requestCountTracker.clear();
Maybe shove these two lines into a helper?

[Nate Chapin, 2014-10-21 22:00:09]

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
File Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
Source/core/fetch/ResourceFetcher.cpp:1155: void
ResourceFetcher::didLoadResource(Resource* resource)
On 2014/10/20 19:28:04, dcheng wrote:
> This can be const now, right?

We can actually drop the Resource* parameter entirely. Done.

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
File Source/core/fetch/ResourceLoader.cpp (right):

https://codereview.chromium.org/561813003/diff/120001/Source/core/fetch/Resou...
Source/core/fetch/ResourceLoader.cpp:237: m_requestCountTracker.clear();
On 2014/10/20 19:28:04, dcheng wrote:
> Maybe shove these two lines into a helper?

Done.

[dcheng, 2014-10-23 09:01:49]

LGTM.

Even after looking over this and our discussions, I'm not sure I understand 100%
of this patch, but overall it looks reasonable. And getting rid of
ENABLE_LOAD_COMPLETION_HACKS will be really, really nice.

https://codereview.chromium.org/561813003/diff/140001/Source/core/fetch/Resou...
File Source/core/fetch/Resource.h (right):

https://codereview.chromium.org/561813003/diff/140001/Source/core/fetch/Resou...
Source/core/fetch/Resource.h:178: void setLoadFinishTime(double finishTime) {
m_loadFinishTime = finishTime; }
Do we have a bug that lets us track when we'll be able to clean up these hacks?
Or is that going to happen relatively quickly after these patches?

[Nate Chapin, 2014-10-23 18:52:55]

On 2014/10/23 09:01:49, dcheng wrote:
> LGTM.
> 
> Even after looking over this and our discussions, I'm not sure I understand
100%
> of this patch, but overall it looks reasonable. And getting rid of
> ENABLE_LOAD_COMPLETION_HACKS will be really, really nice.
> 
>
https://codereview.chromium.org/561813003/diff/140001/Source/core/fetch/Resou...
> File Source/core/fetch/Resource.h (right):
> 
>
https://codereview.chromium.org/561813003/diff/140001/Source/core/fetch/Resou...
> Source/core/fetch/Resource.h:178: void setLoadFinishTime(double finishTime) {
> m_loadFinishTime = finishTime; }
> Do we have a bug that lets us track when we'll be able to clean up these
hacks?
> Or is that going to happen relatively quickly after these patches?

I hope it'll be quick, but I'll open a bug just for completeness.

[Nate Chapin, 2014-10-23 18:52:59]

On 2014/10/23 09:01:49, dcheng wrote:
> LGTM.
> 
> Even after looking over this and our discussions, I'm not sure I understand
100%
> of this patch, but overall it looks reasonable. And getting rid of
> ENABLE_LOAD_COMPLETION_HACKS will be really, really nice.
> 
>
https://codereview.chromium.org/561813003/diff/140001/Source/core/fetch/Resou...
> File Source/core/fetch/Resource.h (right):
> 
>
https://codereview.chromium.org/561813003/diff/140001/Source/core/fetch/Resou...
> Source/core/fetch/Resource.h:178: void setLoadFinishTime(double finishTime) {
> m_loadFinishTime = finishTime; }
> Do we have a bug that lets us track when we'll be able to clean up these
hacks?
> Or is that going to happen relatively quickly after these patches?

I hope it'll be quick, but I'll open a bug just for completeness.

[Nate Chapin, 2014-10-23 19:07:21]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2014-10-23 19:08:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/561813003/140001

[commit-bot: I haz the power, 2014-10-23 21:02:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-23 21:02:32]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/28337)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/33348)

[Nate Chapin, 2014-10-23 21:03:40]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2014-10-23 21:04:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/561813003/140001

[commit-bot: I haz the power, 2014-10-23 22:56:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-23 22:56:55]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/33364)

[Nate Chapin, 2014-10-23 22:58:41]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2014-10-23 23:00:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/561813003/140001

[commit-bot: I haz the power, 2014-10-24 00:44:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-24 00:44:15]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/28367)

[Nate Chapin, 2014-10-24 18:56:24]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2014-10-24 18:57:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/561813003/160001

[commit-bot: I haz the power, 2014-10-24 21:26:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-24 21:26:30]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/33533)

[Nate Chapin, 2014-10-24 21:27:03]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2014-10-24 21:27:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/561813003/160001

[commit-bot: I haz the power, 2014-10-24 22:54:09]

Committed patchset #9 (id:160001) as 184389

[Avi (use Gerrit), 2014-10-25 20:29:13]

On 2014/10/24 22:54:09, I haz the power (commit-bot) wrote:
> Committed patchset #9 (id:160001) as 184389

Reverting in https://codereview.chromium.org/679863002 on suspicions that it's
causing all the RSS test failures on the ASAN bots.

[Avi (use Gerrit), 2014-10-25 23:48:43]

On 2014/10/25 20:29:13, Avi wrote:
> On 2014/10/24 22:54:09, I haz the power (commit-bot) wrote:
> > Committed patchset #9 (id:160001) as 184389
> 
> Reverting in https://codereview.chromium.org/679863002 on suspicions that it's
> causing all the RSS test failures on the ASAN bots.

It appears that this indeed was the issue; with the revert and a roll, the bots
are green.