Bug 74273 – Constant values to set "distanceModel" are undefined

• Component: Web Audio
• Status: RESOLVED FIXED

The constants required to set an specific distance model on an AudioPannerNode are not defined, there is a comment "TODO: add constants" on the specification document:

    https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/specification.html#attributes-AudioPannerNode

Looking at the objects created at runtime there are no properties defined that could be used to set a required model. At the moment you have to look at the source code to figure out what values set which model.

[Raymond Toy]

Created attachment 118890 [details]
Patch

[Adam Barth]

Comment on attachment 118890 [details]
Patch

This patch requires tests.

[Chris Rogers]

Comment on attachment 118890 [details]
Patch

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

> Source/WebCore/webaudio/AudioPannerNode.idl:39
> +        const unsigned short EXPONENTIALDISTANCE = 2;

Can we add an "_" before "DISTANCE"?

[Raymond Toy]

Created attachment 119037 [details]
Patch

[Raymond Toy]

Comment on attachment 118890 [details]
Patch

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

>> Source/WebCore/webaudio/AudioPannerNode.idl:39
>> +        const unsigned short EXPONENTIALDISTANCE = 2;
> 
> Can we add an "_" before "DISTANCE"?

Done.

[WebKit Review Bot]

Comment on attachment 119037 [details]
Patch

Attachment 119037 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10850436

[Chris Rogers]

Looks good to me.

I just checked the OpenAL names and it looks like we're somewhat consistent:


#define AL_DISTANCE_MODEL                         0xD000
#define AL_INVERSE_DISTANCE                       0xD001
#define AL_INVERSE_DISTANCE_CLAMPED               0xD002
#define AL_LINEAR_DISTANCE                        0xD003
#define AL_LINEAR_DISTANCE_CLAMPED                0xD004
#define AL_EXPONENT_DISTANCE                      0xD005
#define AL_EXPONENT_DISTANCE_CLAMPED              0xD006

By the way, it looks like the "clamped" versions need to be handled by a separate bool attribute called "distanceClamped" so we wouldn't define specific constants for them...

[Chris Rogers]

Adam is right that we need tests

[Raymond Toy]

(In reply to comment #8)
> Adam is right that we need tests

What should the tests test?  That the constants are correct?

[Chris Rogers]

No, I think we need proper layout tests that each distance model is calculating the expected attenuation now that the distance models are being formally exposed as an API.  I can explain to you offline how we can effectively do this.

[Raymond Toy]

(In reply to comment #10)
> No, I think we need proper layout tests that each distance model is calculating the expected attenuation now that the distance models are being formally exposed as an API.  I can explain to you offline how we can effectively do this.

The behavior of the existing code is unchanged by this patch, so I think that's outside the scope of this particular patch.  I do agree that tests need to be written, as a different bug.

[Adam Barth]

Comment on attachment 119037 [details]
Patch

At a minimum, you need to write tests that check that these constants are exposed with the correct values.

[Adam Barth]

As a general rule, WebKit does not accept patches that change web-visible behavior without tests demonstrating that change.  Specifically, this patch makes some constants visible to JavaScript, so we need a test that demonstrates that.

[Raymond Toy]

Created attachment 121724 [details]
Patch

[Raymond Toy]

Tests added, but the tests can't pass until bug 75767 is landed.

[WebKit Review Bot]

Comment on attachment 121724 [details]
Patch

Attachment 121724 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11113642

New failing tests:
webaudio/distance-inverse.html
webaudio/distance-linear.html
webaudio/distance-exponential.html

[Chris Rogers]

Comment on attachment 121724 [details]
Patch

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

Nice tests!  Just a couple of comments to simplify + nits

> Source/WebCore/webaudio/AudioPannerNode.idl:39
> +        const unsigned short EXPONENTIAL_DISTANCE = 2;

We need a layout test specifically to test the "distance-constants.html" to test that the constant names match the expected number.

Something like:

// test that ... panner.LINEAR_DISTANCE == 0
// test that ... panner.INVERSE_DISTANCE == 1
// test that ... panner.EXPONENTIAL_DISTANCE == 2

> LayoutTests/webaudio/distance-exponential.html:33
> +      var time;

Lines 16:33 appear to be duplicated in all three .html test files.  Can we move these lines to the .js file as a simplification?

> LayoutTests/webaudio/distance-exponential.html:53
> +          context.oncomplete = checkDistanceResult(tempPanner.EXPONENTIAL_DISTANCE, threshold, false);

Indentation seems off here.

> LayoutTests/webaudio/distance-exponential.html:55
> +      }

Lines 43:54 appear to be duplicated in all three .html test *except* for the distance model constant.  This could be made a function in the .js file which takes the specific constant to test.
This will simplify the .html files quite a bit.

> LayoutTests/webaudio/resources/distance-model-testing.js:21
> +  var gain = (1 - rolloff*(distance - panner.refDistance)/(panner.maxDistance - panner.refDistance));

WebKit style nit: spacing between * and / operators here and in similar places

[Raymond Toy]

Comment on attachment 121724 [details]
Patch

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

>> Source/WebCore/webaudio/AudioPannerNode.idl:39
>> +        const unsigned short EXPONENTIAL_DISTANCE = 2;
> 
> We need a layout test specifically to test the "distance-constants.html" to test that the constant names match the expected number.
> 
> Something like:
> 
> // test that ... panner.LINEAR_DISTANCE == 0
> // test that ... panner.INVERSE_DISTANCE == 1
> // test that ... panner.EXPONENTIAL_DISTANCE == 2

These values are indirectly tested because each tests prints out the model constant as a number which is compared against the expected result.

Do you still want a separate test for that?

[Raymond Toy]

Created attachment 121867 [details]
Patch

[Raymond Toy]

Comment on attachment 121724 [details]
Patch

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

>>> Source/WebCore/webaudio/AudioPannerNode.idl:39
>>> +        const unsigned short EXPONENTIAL_DISTANCE = 2;
>> 
>> We need a layout test specifically to test the "distance-constants.html" to test that the constant names match the expected number.
>> 
>> Something like:
>> 
>> // test that ... panner.LINEAR_DISTANCE == 0
>> // test that ... panner.INVERSE_DISTANCE == 1
>> // test that ... panner.EXPONENTIAL_DISTANCE == 2
> 
> These values are indirectly tested because each tests prints out the model constant as a number which is compared against the expected result.
> 
> Do you still want a separate test for that?

Latest patch tests each constant now, instead of a separate test.

>> LayoutTests/webaudio/distance-exponential.html:33
>> +      var time;
> 
> Lines 16:33 appear to be duplicated in all three .html test files.  Can we move these lines to the .js file as a simplification?

Done.

>> LayoutTests/webaudio/distance-exponential.html:53
>> +          context.oncomplete = checkDistanceResult(tempPanner.EXPONENTIAL_DISTANCE, threshold, false);
> 
> Indentation seems off here.

Removed inadvertent tabs.

>> LayoutTests/webaudio/distance-exponential.html:55
>> +      }
> 
> Lines 43:54 appear to be duplicated in all three .html test *except* for the distance model constant.  This could be made a function in the .js file which takes the specific constant to test.
> This will simplify the .html files quite a bit.

Done.

>> LayoutTests/webaudio/resources/distance-model-testing.js:21
>> +  var gain = (1 - rolloff*(distance - panner.refDistance)/(panner.maxDistance - panner.refDistance));
> 
> WebKit style nit: spacing between * and / operators here and in similar places

Fixed.

[WebKit Review Bot]

Comment on attachment 121867 [details]
Patch

Attachment 121867 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11198136

New failing tests:
webaudio/distance-inverse.html
webaudio/distance-linear.html
webaudio/distance-exponential.html

[Raymond Toy]

Created attachment 122089 [details]
Patch

[Raymond Toy]

Minor update to include audio-testing.js and to fix the expected results to include a line about having the right value for the constants.

[WebKit Review Bot]

Comment on attachment 122089 [details]
Patch

Attachment 122089 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11212172

New failing tests:
webaudio/distance-inverse.html
webaudio/distance-linear.html
webaudio/distance-exponential.html

[Chris Rogers]

Comment on attachment 122089 [details]
Patch

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

> LayoutTests/webaudio/distance-exponential.html:28
> +          // Threshold experimentally determined.

Can you use a more descriptive variable name and comment than simply threshold.  What type of value is being compared, etc...

> LayoutTests/webaudio/distance-inverse.html:29
> +          var threshold = 1.3e-7;

Now that I look at this, let's just settle on a single threshold value for all three tests (I guess the largest of the values).  And then get rid of this code in all the .html files and remove the "threshold" parameter
to createTestAndRun()

> LayoutTests/webaudio/resources/distance-model-testing.js:3
> +var renderLengthSeconds = 8;

Why do we need to render 8 seconds?  That seems like a really long time and wasteful.  For the convolution test, it makes more sense, but this could be fractions of a second, test could complete more quickly, use less memory, etc.

> LayoutTests/webaudio/resources/distance-model-testing.js:5
> +var pulseLengthFrames = pulseLengthSeconds * sampleRate;

lines 4:5 look like they can be removed

> LayoutTests/webaudio/resources/distance-model-testing.js:21
> +function createImpulseBuffer(context, sampleFrameLength) {

Seems like this function would be nice to put in audio-testing.js since it seems to be useful for several different tests.

> LayoutTests/webaudio/resources/distance-model-testing.js:35
> +// spec, not the code.

Instead, can we say that the Web Audio spec follows the OpenAL formulas and provide a link?

> LayoutTests/webaudio/resources/distance-model-testing.js:54
> +function inverseDistance(panner, x, y, z) {

nit: the order these functions are defined is different than the distanceModelFunction array below -- slightly confusing

> LayoutTests/webaudio/resources/distance-model-testing.js:72
> +    impulse = createImpulseBuffer(context, pulseLengthFrames);

Why are we creating a separate impulse for each bufferSource?  Please create just one above the loop

> LayoutTests/webaudio/resources/distance-model-testing.js:88
> +  for (var k = 0; k < nodeCount; ++k) {

Please consolidate code from loop on line 70 into this loop.  There's no need to have two separate loops.  Then your comments 76:85 can move down to just above 89

> LayoutTests/webaudio/resources/distance-model-testing.js:93
> +    // distance.

strange line wrapping - nearby line 94 is long enough to not wrap this comment

> LayoutTests/webaudio/resources/distance-model-testing.js:100
> +  for (var k = 0; k < nodeCount; ++k) {

Once again following my comment on line 88, please consolidate this third loop into the above loop

> LayoutTests/webaudio/resources/distance-model-testing.js:111
> +    bufferSource[k].noteOn(time[k]);

Please just consolidate startSources() function into the above createGraph() function, and you can use the same loop above.

This will read more smoothly having a single loop will all of the graph connection, and the noteOn() one line after the next instead of split out into separate functions and loops

> LayoutTests/webaudio/resources/distance-model-testing.js:120
> +  startSources();

As per comment above, please just consolidate startSources() into the createGraph() function

> LayoutTests/webaudio/resources/distance-model-testing.js:122
> +  context.oncomplete = checkDistanceResult(distanceModel, expectedModel, threshold, false);

Please remove final argument (false) since we can simply comment out lines for the bug below

> LayoutTests/webaudio/resources/distance-model-testing.js:132
> +function checkDistanceResult(model, expectedModel, threshold, debug) {

Please remove "debug" param

> LayoutTests/webaudio/resources/distance-model-testing.js:142
> +      testPassed("Distance model value matched expected value.");

Please update text for failure case -- it's the same text as the passed case

> LayoutTests/webaudio/resources/distance-model-testing.js:151
> +    // (For debugging.)

remove (for debugging) comment

> LayoutTests/webaudio/resources/distance-model-testing.js:154
> +    // expected location.  (For debugging.)

remove (for debugging) comment

> LayoutTests/webaudio/resources/distance-model-testing.js:158
> +    // so we can find where our impulses are.

"impulses" -> "distance-attenuated impulses"

I'd add a further comment here saying that we then check each of these values with the expected distance attenuation for that distance

> LayoutTests/webaudio/resources/distance-model-testing.js:163
> +        expected *= equalPowerGain();

Please add comment explaining that this compensates for the "center-panning" value using the EQUALPOWERPANNING model

> LayoutTests/webaudio/resources/distance-model-testing.js:165
> +        var error = Math.abs(renderedData[k] - expected)/Math.abs(expected);

nit: spacing with /

> LayoutTests/webaudio/resources/distance-model-testing.js:168
> +        }

Let's get rid of this debug code for code we actually commit

> LayoutTests/webaudio/resources/distance-model-testing.js:170
> +        if (k != Math.round(sampleRate * time[impulseCount])) {

I'd add a comment before line 170, saying that we keep track of the exact sample-frame offsets compared with their expected locations

> LayoutTests/webaudio/resources/distance-model-testing.js:179
> +      testPassed("Number of nodes is correct.");

Can we be a bit more specific what this test is, something like:

'Number of impulses found matches number of panner nodes"

> LayoutTests/webaudio/resources/distance-model-testing.js:196
> +    // FIXME:  File a bug about this.

We actually have a specific bug for this now, so we can provide the exact link

> LayoutTests/webaudio/resources/distance-model-testing.js:198
> +      console.log(timeErrors.length + " timing errors found");

Please convert from console.log() format to testPassed/testFailed format, get rid of the "debug" parameter to this function and simply comment out these specific tests

Then once the bug is fixed, these lines can simply be uncommented and the expected results .txt files updated

[Raymond Toy]

Comment on attachment 122089 [details]
Patch

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

>> LayoutTests/webaudio/distance-inverse.html:29
>> +          var threshold = 1.3e-7;
> 
> Now that I look at this, let's just settle on a single threshold value for all three tests (I guess the largest of the values).  And then get rid of this code in all the .html files and remove the "threshold" parameter
> to createTestAndRun()

They vary a bit (from 1.3e-7 to 2.3e-6).  Do you want to use just one?

>> LayoutTests/webaudio/resources/distance-model-testing.js:72
>> +    impulse = createImpulseBuffer(context, pulseLengthFrames);
> 
> Why are we creating a separate impulse for each bufferSource?  Please create just one above the loop

Is it possible to turn a single bufferSource on at multiple times?

[Raymond Toy]

Comment on attachment 122089 [details]
Patch

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

>>> LayoutTests/webaudio/resources/distance-model-testing.js:72
>>> +    impulse = createImpulseBuffer(context, pulseLengthFrames);
>> 
>> Why are we creating a separate impulse for each bufferSource?  Please create just one above the loop
> 
> Is it possible to turn a single bufferSource on at multiple times?

Sorry, I misread your comment.  Only one impulse is needed.

[Chris Rogers]

(In reply to comment #26)
> (From update of attachment 122089 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=122089&action=review
> 
> >> LayoutTests/webaudio/distance-inverse.html:29
> >> +          var threshold = 1.3e-7;
> > 
> > Now that I look at this, let's just settle on a single threshold value for all three tests (I guess the largest of the values).  And then get rid of this code in all the .html files and remove the "threshold" parameter
> > to createTestAndRun()
> 
> They vary a bit (from 1.3e-7 to 2.3e-6).  Do you want to use just one?

Yes, I was hoping you could pick a threshold which is the least stringent of them all and go with that, and then just define that threshold inside the .js file instead of separately for each .html

[Raymond Toy]

(In reply to comment #28)
> (In reply to comment #26)
> > (From update of attachment 122089 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=122089&action=review
> > 
> > >> LayoutTests/webaudio/distance-inverse.html:29
> > >> +          var threshold = 1.3e-7;
> > > 
> > > Now that I look at this, let's just settle on a single threshold value for all three tests (I guess the largest of the values).  And then get rid of this code in all the .html files and remove the "threshold" parameter
> > > to createTestAndRun()
> > 
> > They vary a bit (from 1.3e-7 to 2.3e-6).  Do you want to use just one?
> 
> Yes, I was hoping you could pick a threshold which is the least stringent of them all and go with that, and then just define that threshold inside the .js file instead of separately for each .html

I'll update this once the bug 75767 has landed due to the change in audio-testing.js.

[Raymond Toy]

Created attachment 122818 [details]
Patch

[Raymond Toy]

Comment on attachment 122089 [details]
Patch

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

>> LayoutTests/webaudio/distance-exponential.html:28
>> +          // Threshold experimentally determined.
> 
> Can you use a more descriptive variable name and comment than simply threshold.  What type of value is being compared, etc...

Done.

>>>>> LayoutTests/webaudio/distance-inverse.html:29
>>>>> +          var threshold = 1.3e-7;
>>>> 
>>>> Now that I look at this, let's just settle on a single threshold value for all three tests (I guess the largest of the values).  And then get rid of this code in all the .html files and remove the "threshold" parameter
>>>> to createTestAndRun()
>>> 
>>> They vary a bit (from 1.3e-7 to 2.3e-6).  Do you want to use just one?
>> 
>> Yes, I was hoping you could pick a threshold which is the least stringent of them all and go with that, and then just define that threshold inside the .js file instead of separately for each .html
> 
> I'll update this once the bug 75767 has landed due to the change in audio-testing.js.

Done.

>> LayoutTests/webaudio/resources/distance-model-testing.js:3
>> +var renderLengthSeconds = 8;
> 
> Why do we need to render 8 seconds?  That seems like a really long time and wasteful.  For the convolution test, it makes more sense, but this could be fractions of a second, test could complete more quickly, use less memory, etc.

Too much copying and pasting from the convolution test.  The time is much smaller now.

>> LayoutTests/webaudio/resources/distance-model-testing.js:5
>> +var pulseLengthFrames = pulseLengthSeconds * sampleRate;
> 
> lines 4:5 look like they can be removed

Line 4 deleted, but 5 is used to set the length of the impulse.

>> LayoutTests/webaudio/resources/distance-model-testing.js:21
>> +function createImpulseBuffer(context, sampleFrameLength) {
> 
> Seems like this function would be nice to put in audio-testing.js since it seems to be useful for several different tests.

Already done in the equalpower panner test.

>> LayoutTests/webaudio/resources/distance-model-testing.js:35
>> +// spec, not the code.
> 
> Instead, can we say that the Web Audio spec follows the OpenAL formulas and provide a link?

Done.

>> LayoutTests/webaudio/resources/distance-model-testing.js:54
>> +function inverseDistance(panner, x, y, z) {
> 
> nit: the order these functions are defined is different than the distanceModelFunction array below -- slightly confusing

That was the order of the implementation of the tests.  Order is changed now.

>> LayoutTests/webaudio/resources/distance-model-testing.js:88
>> +  for (var k = 0; k < nodeCount; ++k) {
> 
> Please consolidate code from loop on line 70 into this loop.  There's no need to have two separate loops.  Then your comments 76:85 can move down to just above 89

Done.

>> LayoutTests/webaudio/resources/distance-model-testing.js:93
>> +    // distance.
> 
> strange line wrapping - nearby line 94 is long enough to not wrap this comment

Fixed.

>> LayoutTests/webaudio/resources/distance-model-testing.js:100
>> +  for (var k = 0; k < nodeCount; ++k) {
> 
> Once again following my comment on line 88, please consolidate this third loop into the above loop

Done.

>> LayoutTests/webaudio/resources/distance-model-testing.js:111
>> +    bufferSource[k].noteOn(time[k]);
> 
> Please just consolidate startSources() function into the above createGraph() function, and you can use the same loop above.
> 
> This will read more smoothly having a single loop will all of the graph connection, and the noteOn() one line after the next instead of split out into separate functions and loops

Done

>> LayoutTests/webaudio/resources/distance-model-testing.js:120
>> +  startSources();
> 
> As per comment above, please just consolidate startSources() into the createGraph() function

Done

>> LayoutTests/webaudio/resources/distance-model-testing.js:132
>> +function checkDistanceResult(model, expectedModel, threshold, debug) {
> 
> Please remove "debug" param

Done

>> LayoutTests/webaudio/resources/distance-model-testing.js:142
>> +      testPassed("Distance model value matched expected value.");
> 
> Please update text for failure case -- it's the same text as the passed case

Done.

>> LayoutTests/webaudio/resources/distance-model-testing.js:154
>> +    // expected location.  (For debugging.)
> 
> remove (for debugging) comment

Done.

>> LayoutTests/webaudio/resources/distance-model-testing.js:163
>> +        expected *= equalPowerGain();
> 
> Please add comment explaining that this compensates for the "center-panning" value using the EQUALPOWERPANNING model

Done.

>> LayoutTests/webaudio/resources/distance-model-testing.js:165
>> +        var error = Math.abs(renderedData[k] - expected)/Math.abs(expected);
> 
> nit: spacing with /

Fixed.

>> LayoutTests/webaudio/resources/distance-model-testing.js:168
>> +        }
> 
> Let's get rid of this debug code for code we actually commit

Removed.

>> LayoutTests/webaudio/resources/distance-model-testing.js:179
>> +      testPassed("Number of nodes is correct.");
> 
> Can we be a bit more specific what this test is, something like:
> 
> 'Number of impulses found matches number of panner nodes"

Done.

>> LayoutTests/webaudio/resources/distance-model-testing.js:196
>> +    // FIXME:  File a bug about this.
> 
> We actually have a specific bug for this now, so we can provide the exact link

The noteOn bug has been commited so this comment and the following code has been updated appropriately.

>> LayoutTests/webaudio/resources/distance-model-testing.js:198
>> +      console.log(timeErrors.length + " timing errors found");
> 
> Please convert from console.log() format to testPassed/testFailed format, get rid of the "debug" parameter to this function and simply comment out these specific tests
> 
> Then once the bug is fixed, these lines can simply be uncommented and the expected results .txt files updated

Done

[Chris Rogers]

Looks good to me.

[Raymond Toy]

(In reply to comment #32)
> Looks good to me.

Unfortunately, the tests don't pass on Windows.  I have no idea why, but the expected values are way off.  but we know the panner is working because the panner demos sound just fine.  Yuck.

[Raymond Toy]

Wait for bug 76659 to land before continuing with this.

[Eric Seidel (no email)]

Comment on attachment 121724 [details]
Patch

Cleared review? from obsolete attachment 121724 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).

[Eric Seidel (no email)]

Comment on attachment 122089 [details]
Patch

Cleared review? from obsolete attachment 122089 [details] so that this bug does not appear in http://webkit.org/pending-review.  If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).

[Raymond Toy]

Created attachment 124760 [details]
Patch

[Raymond Toy]

Now that the fix for bug 76659 has landed, this test has been updated to use the new timeToSampleFrame function to get the correct expected sample frame for each impulse.   Manually ran tests on OSX and Windows, where they pass.

[WebKit Review Bot]

Attachment 124760 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
From git://git.webkit.org/WebKit
   a72c8a9..11be23e  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 106364 = a72c8a9b2ed7c202860de7e4733e96d7f60cdc14
r106366 = 11be23e25c7011a2675e260f91a1b3cbfa7052f9
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Raymond Toy]

Created attachment 124763 [details]
Patch

[WebKit Review Bot]

Attachment 124763 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Raymond Toy]

Created attachment 124774 [details]
Patch

[WebKit Review Bot]

Attachment 124774 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Raymond Toy]

Created attachment 124778 [details]
Patch

[WebKit Review Bot]

Attachment 124778 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Raymond Toy]

Created attachment 125020 [details]
Patch

[Chris Rogers]

Comment on attachment 125020 [details]
Patch

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

> LayoutTests/webaudio/resources/distance-model-testing.js:201
> +        // following if it enable this test.

Please land 75996 first and remove the comments about these bugs (lines 199:201)

[Raymond Toy]

Created attachment 125038 [details]
Patch

[Chris Rogers]

Looks good.

[Kenneth Russell]

Comment on attachment 125038 [details]
Patch

rs=me

[Raymond Toy]

Comment on attachment 125038 [details]
Patch

Thanks for the review.

[WebKit Review Bot]

Comment on attachment 125038 [details]
Patch

Clearing flags on attachment: 125038

Committed r106580: <http://trac.webkit.org/changeset/106580>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.