Bug 74558 – [chromium] DatabaseObserver needs threadsafe fixes and other clean-up.

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[David Levin]

See summary.

[David Levin]

Created attachment 119324 [details]
Patch

[Dmitry Titov]

Comment on attachment 119324 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=119324&action=review

> Source/WebKit/chromium/src/DatabaseObserver.cpp:90
> +        m_workerLoaderProxy->postTaskForModeToWorkerContext(

do we follow this style in WebKit or should it be in one long line?

> LayoutTests/http/tests/workers/interrupt-database-sync-open-crash.html:8
> +var workers = new Array(30);

why 30 workers and 10 iterations? Would be nice to add a comment on the number selection.

[David Levin]

Created attachment 119360 [details]
Patch for landing

[David Levin]

Created attachment 119361 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 119361 [details]
Patch for landing

Clearing flags on attachment: 119361

Committed r102894: <http://trac.webkit.org/changeset/102894>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

The test you landed with this patch is failing on Mac builds because there is no expected result. But when I put in an expected result, I get different results on different runs. It seems the output is nondeterministic. How is this supposed to work with the test machinery?

[David Levin]

(In reply to comment #7)
> The test you landed with this patch is failing on Mac builds because there is no expected result.
Yeah, I realized that after I landed it and then started to modify it.

But I didn't realize it was causing a failure.

> But when I put in an expected result, I get different results on different runs. It seems the output is nondeterministic. How is this supposed to work with the test machinery?

I see. It is the exception. I should put in a handler for exceptions (since it isn't important) and causes the indeterminate output when it is unhandled.

[David Levin]

(In reply to comment #8)
> (In reply to comment #7)
> > The test you landed with this patch is failing on Mac builds because there is no expected result.
> Yeah, I realized that after I landed it and then started to modify it.
> 
> But I didn't realize it was causing a failure.
> 
> > But when I put in an expected result, I get different results on different runs. It seems the output is nondeterministic. How is this supposed to work with the test machinery?
> 
> I see. It is the exception. I should put in a handler for exceptions (since it isn't important) and causes the indeterminate output when it is unhandled.

btw, I'm off tomorrow but I'm around on Tuesday. I'll see if I can fix this tomorrow night or on Tuesday.

If you want a fix before then, feel free to roll out my fix and I'll land it all again with a result that is determinate. Sorry I can't handler it sooner.

[David Levin]

(In reply to comment #7)
> The test you landed with this patch is failing on Mac builds because there is no expected result. But when I put in an expected result, I get different results on different runs. It seems the output is nondeterministic. How is this supposed to work with the test machinery?

Skipped the test for now as I didn't figure out a quick fix (r103394).