Bug 58459 - Exclude webkitGetUserMedia from the navigator-detached-no-crash test
Summary: Exclude webkitGetUserMedia from the navigator-detached-no-crash test
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Leandro Graciá Gil
URL:
Keywords:
Depends on: 56586
Blocks: 56459
  Show dependency treegraph
 
Reported: 2011-04-13 11:09 PDT by Leandro Graciá Gil
Modified: 2012-09-19 07:18 PDT (History)
2 users (show)

See Also:


Attachments
Patch (2.00 KB, patch)
2011-04-13 11:15 PDT, Leandro Graciá Gil
no flags Details | Formatted Diff | Diff
Patch (2.24 KB, patch)
2011-04-13 11:35 PDT, Leandro Graciá Gil
no flags Details | Formatted Diff | Diff
Patch (1.93 KB, patch)
2011-04-13 11:45 PDT, Leandro Graciá Gil
jianli: review+
jianli: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Leandro Graciá Gil 2011-04-13 11:09:20 PDT
Exclude the webkitGetUserMedia method from the test until it is supported by most platforms. Once this happens, the expected behaviour for this method is to throw an exception when invoked with no arguments.
Comment 1 Leandro Graciá Gil 2011-04-13 11:15:15 PDT
Created attachment 89408 [details]
Patch
Comment 2 Jian Li 2011-04-13 11:25:30 PDT
Comment on attachment 89408 [details]
Patch

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

> LayoutTests/fast/dom/navigator-detached-no-crash.html:56
> +      if (p != 'webkitGetUserMedia') { // Don't include the Media Stream API functions until most platforms have support.

Can you combine this logic with the existing check for "geolocation" into some common check? Maybe you can keep an excluding list and check this list in for loop.
Comment 3 Leandro Graciá Gil 2011-04-13 11:35:32 PDT
Created attachment 89412 [details]
Patch
Comment 4 Leandro Graciá Gil 2011-04-13 11:35:52 PDT
Comment on attachment 89408 [details]
Patch

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

>> LayoutTests/fast/dom/navigator-detached-no-crash.html:56
>> +      if (p != 'webkitGetUserMedia') { // Don't include the Media Stream API functions until most platforms have support.
> 
> Can you combine this logic with the existing check for "geolocation" into some common check? Maybe you can keep an excluding list and check this list in for loop.

Fixed.
Comment 5 Jian Li 2011-04-13 11:41:46 PDT
Comment on attachment 89412 [details]
Patch

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

> LayoutTests/fast/dom/navigator-detached-no-crash.html:55
> +    if (p != 'geolocation' && p != 'webkitGetUserMedia') { // Don't include Geolocation or the Media Stream API functions until most platforms have support.

Better to use early return, like:
  if (p == 'geolocation' || p == 'webkitGetUserMedia') //...
    continue;
  ...
Comment 6 Leandro Graciá Gil 2011-04-13 11:45:18 PDT
Created attachment 89417 [details]
Patch
Comment 7 Leandro Graciá Gil 2011-04-13 11:45:34 PDT
Comment on attachment 89412 [details]
Patch

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

>> LayoutTests/fast/dom/navigator-detached-no-crash.html:55
>> +    if (p != 'geolocation' && p != 'webkitGetUserMedia') { // Don't include Geolocation or the Media Stream API functions until most platforms have support.
> 
> Better to use early return, like:
>   if (p == 'geolocation' || p == 'webkitGetUserMedia') //...
>     continue;
>   ...

Fixed.
Comment 8 Jian Li 2011-04-13 12:09:28 PDT
Committed as https://trac.webkit.org/changeset/83754.