Skip to content
Permalink
Browse files
Make Document::postTask to use a single queue of tasks, to fire them …
…in order

https://bugs.webkit.org/show_bug.cgi?id=35943

Reviewed by Darin Adler.

JavaScriptCore:

The patch uses CFRunLoopTimer to schedule execution of tasks instead of performSelectorOnMainThread which apparently can starve other event sources.
The timer is used when the schedule request is coming on the main thread itself. This happens when the task is posted on the main thread or
when too many tasks are posted and the queue does 'stop and re-schedule' to make sure run loop has a chance to execute other events.

* wtf/mac/MainThreadMac.mm:
(WTF::timerFired):
(WTF::postTimer):
(WTF::scheduleDispatchFunctionsOnMainThread): Use timer posted to the current RunLoop if scheduling the task execution while on the main thread.

WebCore:

Test: existing worker-cloneport.html which was broken by initial patch in http://trac.webkit.org/changeset/55593.
Additional test which indirectly verifies the order of execution will come as part of https://bugs.webkit.org/show_bug.cgi?id=34726

* dom/Document.cpp:
(WebCore::Document::postTask): Always use the same task queue, independent of what thread is posting the task.

LayoutTests:

Updated the test since the order of events coming from independent task sources has changed.
This is a behavior change, but is consistent with the spec (which specifically says these events may be fired
in any order) and the last version of FF (3.5.8) which was completing the test successfully.

* http/tests/appcache/top-frame-2-expected.txt:
* http/tests/appcache/top-frame-2.html:

Canonical link: https://commits.webkit.org/47111@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@55816 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Dmitry Titov committed Mar 11, 2010
1 parent 4328d5d commit da33bb7b5a67582d25ae26633240517899cb2dca
@@ -1,3 +1,19 @@
2010-03-10 Dmitry Titov <dimich@chromium.org>

Reviewed by Darin Adler.

Make Document::postTask to use a single queue of tasks, to fire them in order
https://bugs.webkit.org/show_bug.cgi?id=35943

The patch uses CFRunLoopTimer to schedule execution of tasks instead of performSelectorOnMainThread which apparently can starve other event sources.
The timer is used when the schedule request is coming on the main thread itself. This happens when the task is posted on the main thread or
when too many tasks are posted and the queue does 'stop and re-schedule' to make sure run loop has a chance to execute other events.

* wtf/mac/MainThreadMac.mm:
(WTF::timerFired):
(WTF::postTimer):
(WTF::scheduleDispatchFunctionsOnMainThread): Use timer posted to the current RunLoop if scheduling the task execution while on the main thread.

2010-03-10 Geoffrey Garen <ggaren@apple.com>

Windows build fix: added new symbol.
@@ -29,8 +29,10 @@
#import "config.h"
#import "MainThread.h"

#import <CoreFoundation/CoreFoundation.h>
#import <Foundation/NSThread.h>
#import <wtf/Assertions.h>
#import <wtf/Threading.h>

@interface WTFMainThreadCaller : NSObject {
}
@@ -63,9 +65,36 @@ void initializeMainThreadPlatform()
#endif
}

static bool isTimerPosted; // This is only accessed on the 'main' thread.

static void timerFired(CFRunLoopTimerRef timer, void*)
{
CFRelease(timer);
isTimerPosted = false;
WTF::dispatchFunctionsFromMainThread();
}

void postTimer()
{
ASSERT(isMainThread());

if (isTimerPosted)
return;

isTimerPosted = true;
CFRunLoopAddTimer(CFRunLoopGetCurrent(), CFRunLoopTimerCreate(0, 0, 0, 0, 0, timerFired, 0), kCFRunLoopCommonModes);
}


void scheduleDispatchFunctionsOnMainThread()
{
ASSERT(staticMainThreadCaller);

if (isMainThread()) {
postTimer();
return;
}

#if USE(WEB_THREAD)
[staticMainThreadCaller performSelector:@selector(call) onThread:webThread withObject:nil waitUntilDone:NO];
#else
@@ -1,3 +1,17 @@
2010-03-10 Dmitry Titov <dimich@chromium.org>

Reviewed by Darin Adler.

Make Document::postTask to use a single queue of tasks, to fire them in order
https://bugs.webkit.org/show_bug.cgi?id=35943

Updated the test since the order of events coming from independent task sources has changed.
This is a behavior change, but is consistent with the spec (which specifically says these events may be fired
in any order) and the last version of FF (3.5.8) which was completing the test successfully.

* http/tests/appcache/top-frame-2-expected.txt:
* http/tests/appcache/top-frame-2.html:

2010-03-10 Antonio Gomes <tonikitoo@webkit.org>

The test was introduced in r55796, it has been failing in Gtk bots since then.
@@ -2,6 +2,5 @@ Test that a subframe without manifest gets picked by a relevant application cach

Should say SUCCESS:

checking
SUCCESS

@@ -16,26 +16,47 @@
document.getElementById("result").innerHTML += message + "<br>";
}

function debug(message)
{
// If running manually in the browser, print the sequence of events.
if (!window.layoutTestController)
log(message);
}

var receivedExpectedMessage = false;
var receivedCheckingEvent = false;
var receivedNoupdateEvent = false;

function test()
{
applicationCache.onnoupdate = null;
applicationCache.oncached = null;

// When a new main resource is associated with the cache, an update should be started.
applicationCache.onchecking = function() { log("checking") }
applicationCache.onchecking = function() { debug("checking"); receivedCheckingEvent = true; checkDone(); }
applicationCache.onnoupdate = function() { debug("noupdate"); receivedNoupdateEvent = true; checkDone(); }

var ifr = document.createElement("iframe");
ifr.setAttribute("src", "resources/subframe-2.html");
document.body.appendChild(ifr);
}

function checkDone()
{
if (receivedExpectedMessage && receivedCheckingEvent && receivedNoupdateEvent) {
log("SUCCESS");
if (window.layoutTestController)
layoutTestController.notifyDone();
}
}

applicationCache.onnoupdate = function() { test() }
applicationCache.oncached = function() { test() }

applicationCache.onupdateready = function() { log("FAIL: received unexpected updateready event") }
applicationCache.onerror = function() { log("FAIL: received unexpected error event") }

window.addEventListener("message", function() { log("SUCCESS"); if (window.layoutTestController) layoutTestController.notifyDone() }, false);
window.addEventListener("message", function() { debug("message"); receivedExpectedMessage = true; checkDone(); }, false);

</script>
</body>
@@ -1,3 +1,16 @@
2010-03-10 Dmitry Titov <dimich@chromium.org>

Reviewed by Darin Adler.

Make Document::postTask to use a single queue of tasks, to fire them in order
https://bugs.webkit.org/show_bug.cgi?id=35943

Test: existing worker-cloneport.html which was broken by initial patch in http://trac.webkit.org/changeset/55593.
Additional test which indirectly verifies the order of execution will come as part of https://bugs.webkit.org/show_bug.cgi?id=34726

* dom/Document.cpp:
(WebCore::Document::postTask): Always use the same task queue, independent of what thread is posting the task.

2010-03-10 Sanjeev Radhakrishnan <sanjeevr@chromium.org>

Reviewed by Darin Fisher.
@@ -4685,25 +4685,6 @@ void Document::scriptImported(unsigned long identifier, const String& sourceStri
#endif
}

class ScriptExecutionContextTaskTimer : public TimerBase {
public:
ScriptExecutionContextTaskTimer(PassRefPtr<Document> context, PassOwnPtr<ScriptExecutionContext::Task> task)
: m_context(context)
, m_task(task)
{
}

private:
virtual void fired()
{
m_task->performTask(m_context.get());
delete this;
}

RefPtr<Document> m_context;
OwnPtr<ScriptExecutionContext::Task> m_task;
};

struct PerformTaskContext : Noncopyable {
PerformTaskContext(PassRefPtr<DocumentWeakReference> documentReference, PassOwnPtr<ScriptExecutionContext::Task> task)
: documentReference(documentReference)
@@ -4730,12 +4711,7 @@ static void performTask(void* ctx)

void Document::postTask(PassOwnPtr<Task> task)
{
if (isMainThread()) {
ScriptExecutionContextTaskTimer* timer = new ScriptExecutionContextTaskTimer(static_cast<Document*>(this), task);
timer->startOneShot(0);
} else {
callOnMainThread(performTask, new PerformTaskContext(m_weakReference, task));
}
callOnMainThread(performTask, new PerformTaskContext(m_weakReference, task));
}

Element* Document::findAnchor(const String& name)

0 comments on commit da33bb7

Please sign in to comment.