WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
145947
Enable MEDIA_STREAM flag to be turned on
https://bugs.webkit.org/show_bug.cgi?id=145947
Summary
Enable MEDIA_STREAM flag to be turned on
Matthew Daiter
Reported
2015-06-12 16:15:37 PDT
Enabling the MEDIA_STREAM flag to be turned on
Attachments
Patch
(68.29 KB, patch)
2015-06-12 16:57 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(68.25 KB, patch)
2015-06-12 17:12 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Updated patch.
(16.70 KB, patch)
2015-06-19 15:50 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
More style-conforming
(16.70 KB, patch)
2015-06-19 15:57 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(31.84 KB, patch)
2015-06-21 00:53 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(28.58 KB, patch)
2015-06-22 11:54 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(29.75 KB, patch)
2015-06-22 12:56 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(31.47 KB, patch)
2015-06-22 13:48 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews100 for mac-mavericks
(571.08 KB, application/zip)
2015-06-22 14:40 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews106 for mac-mavericks-wk2
(604.35 KB, application/zip)
2015-06-22 14:45 PDT
,
Build Bot
no flags
Details
Patch
(44.63 KB, patch)
2015-06-23 11:28 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews104 for mac-mavericks-wk2
(629.37 KB, application/zip)
2015-06-23 12:28 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews101 for mac-mavericks
(579.34 KB, application/zip)
2015-06-23 12:51 PDT
,
Build Bot
no flags
Details
Patch
(41.50 KB, patch)
2015-06-23 12:59 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(43.07 KB, patch)
2015-06-23 13:47 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(67.10 KB, patch)
2015-06-23 17:05 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-mavericks
(582.82 KB, application/zip)
2015-06-23 17:57 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews104 for mac-mavericks-wk2
(629.46 KB, application/zip)
2015-06-23 18:02 PDT
,
Build Bot
no flags
Details
Patch
(86.30 KB, patch)
2015-06-24 10:23 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-mavericks
(585.89 KB, application/zip)
2015-06-24 11:16 PDT
,
Build Bot
no flags
Details
Archive of layout-test-results from ews104 for mac-mavericks-wk2
(676.18 KB, application/zip)
2015-06-24 11:20 PDT
,
Build Bot
no flags
Details
Patch
(81.46 KB, patch)
2015-06-24 13:51 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(81.52 KB, patch)
2015-06-24 14:33 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(81.36 KB, patch)
2015-06-24 16:05 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(81.37 KB, patch)
2015-06-24 16:32 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(54.47 KB, patch)
2015-06-29 16:33 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(78.80 KB, patch)
2015-06-29 17:07 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(80.56 KB, patch)
2015-06-30 10:18 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(80.46 KB, patch)
2015-06-30 12:13 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(81.68 KB, patch)
2015-06-30 12:14 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(83.02 KB, patch)
2015-06-30 12:37 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(83.93 KB, patch)
2015-06-30 15:05 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(83.65 KB, patch)
2015-06-30 17:10 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(58.38 KB, patch)
2015-07-01 10:16 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Patch
(83.81 KB, patch)
2015-07-01 10:24 PDT
,
Matthew Daiter
no flags
Details
Formatted Diff
Diff
Show Obsolete
(34)
View All
Add attachment
proposed patch, testcase, etc.
Matthew Daiter
Comment 1
2015-06-12 16:57:03 PDT
Created
attachment 254834
[details]
Patch
Matthew Daiter
Comment 2
2015-06-12 17:12:12 PDT
Created
attachment 254836
[details]
Patch
Jon Lee
Comment 3
2015-06-16 15:28:05 PDT
Needs rebasing?
Matthew Daiter
Comment 4
2015-06-19 15:50:46 PDT
Created
attachment 255237
[details]
Updated patch.
WebKit Commit Bot
Comment 5
2015-06-19 15:53:32 PDT
Attachment 255237
[details]
did not pass style-queue: ERROR: ChangeLog:5: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Matthew Daiter
Comment 6
2015-06-19 15:57:05 PDT
Created
attachment 255238
[details]
More style-conforming
Matthew Daiter
Comment 7
2015-06-21 00:53:38 PDT
Created
attachment 255314
[details]
Patch
Jon Lee
Comment 8
2015-06-21 16:39:04 PDT
Comment on
attachment 255314
[details]
Patch Build issues in the mock.
Matthew Daiter
Comment 9
2015-06-22 11:54:17 PDT
Created
attachment 255359
[details]
Patch
Matthew Daiter
Comment 10
2015-06-22 12:56:46 PDT
Created
attachment 255363
[details]
Patch
Matthew Daiter
Comment 11
2015-06-22 13:48:22 PDT
Created
attachment 255365
[details]
Patch
Build Bot
Comment 12
2015-06-22 14:40:51 PDT
Comment on
attachment 255365
[details]
Patch
Attachment 255365
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/5267354325876736
New failing tests: js/dom/global-constructors-attributes.html
Build Bot
Comment 13
2015-06-22 14:40:53 PDT
Created
attachment 255371
[details]
Archive of layout-test-results from ews100 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Build Bot
Comment 14
2015-06-22 14:45:42 PDT
Comment on
attachment 255365
[details]
Patch
Attachment 255365
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/5345718164258816
New failing tests: js/dom/global-constructors-attributes.html
Build Bot
Comment 15
2015-06-22 14:45:44 PDT
Created
attachment 255373
[details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Matthew Daiter
Comment 16
2015-06-22 16:50:54 PDT
rdar://problem/21365829
Matthew Daiter
Comment 17
2015-06-22 16:54:09 PDT
Was modifying files, compiled correctly but was told to use Refs instead of RefPtrs to avoid possibilities of null references.
Matthew Daiter
Comment 18
2015-06-23 11:28:50 PDT
Created
attachment 255417
[details]
Patch
WebKit Commit Bot
Comment 19
2015-06-23 11:31:54 PDT
Attachment 255417
[details]
did not pass style-queue: ERROR: Source/WebKit2/ChangeLog:18: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 1 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 20
2015-06-23 12:28:08 PDT
Comment on
attachment 255417
[details]
Patch
Attachment 255417
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/6586288316612608
New failing tests: js/dom/global-constructors-attributes.html
Build Bot
Comment 21
2015-06-23 12:28:10 PDT
Created
attachment 255421
[details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Build Bot
Comment 22
2015-06-23 12:51:15 PDT
Comment on
attachment 255417
[details]
Patch
Attachment 255417
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/4870400563478528
New failing tests: js/dom/global-constructors-attributes.html
Build Bot
Comment 23
2015-06-23 12:51:18 PDT
Created
attachment 255422
[details]
Archive of layout-test-results from ews101 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-mavericks Platform: Mac OS X 10.9.5
Matthew Daiter
Comment 24
2015-06-23 12:59:50 PDT
Created
attachment 255424
[details]
Patch
Jon Lee
Comment 25
2015-06-23 13:15:23 PDT
Comment on
attachment 255424
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=255424&action=review
> ChangeLog:25 > +
Added twice.
> Source/WTF/ChangeLog:4 > + Changed ifdef declaration in FeatureDefines
Our style is to move these ChangeLog specific comments to after the "Reviewed by" line. The script that puts together the final commit message combines these ChangeLogs together, and the way you have them formatted will look more verbose and confusing in the end.
> Source/WTF/wtf/FeatureDefines.h:592 > #endif
Can we remove this and instead update the FeatureDefines.xcconfig files?
> Source/WebKit2/ChangeLog:-25 > -
This shouldn't be here.
> Source/WebKit/mac/ChangeLog:10 > + * WebCoreSupport/WebUserMediaClient.mm: Chagned refs, pointers,
typo: Changed
> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:184 > + // Stub
Please include a FIXME with a link to the bug that fills in the stub.
> LayoutTests/platform/mac-mavericks/js/dom/global-constructors-attributes-expected.txt:820 > +PASS Object.getOwnPropertyDescriptor(global, 'MediaStreamTrackEvent').configurable is true
Shouldn't all of this be in the mac results, not mac-mavericks?
Matthew Daiter
Comment 26
2015-06-23 13:47:29 PDT
Created
attachment 255427
[details]
Patch
Jon Lee
Comment 27
2015-06-23 15:26:03 PDT
Comment on
attachment 255427
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=255427&action=review
> ChangeLog:11 > + * WebKitBuild/Debug/DerivedSources/WebCore: Added.
This ChangeLog shouldn't exist.
> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:144 > +ENABLE_MEDIA_STREAM = ENABLE_MEDIA_STREAM;
This needs to be added to the FEATURE_DEFINES below. There's also a missing ChangeLog file. Please make everything up to the Reviewed by line consistent with the other ChangeLogs.
> Source/WTF/ChangeLog:7 > + Reviewed by NOBODY (OOPS!).
Please make everything up to the Reviewed by line consistent with the other ChangeLogs.
> Source/WebCore/ChangeLog:6 > + <
rdar://problem/21365829
>
Please make everything up to the Reviewed by line consistent with the other ChangeLogs.
> Source/WebCore/Configurations/FeatureDefines.xcconfig:144 > +ENABLE_MEDIA_STREAM = ENABLE_MEDIA_STREAM;
This needs to be added to the FEATURE_DEFINES below.
> Source/WebKit2/ChangeLog:3 > + Enable MEDIA_STREAM. Exported necessary functions to WEBCORE
Please make everything up to the Reviewed by line consistent with the other ChangeLogs.
> Source/WebKit/mac/ChangeLog:8 > +
Missing Reviewed by. Please make everything up to the Reviewed by line consistent with the other ChangeLogs.
> Source/WebKit/mac/Configurations/FeatureDefines.xcconfig:144 > +ENABLE_MEDIA_STREAM = ENABLE_MEDIA_STREAM;
This needs to be added to the FEATURE_DEFINES below.
> LayoutTests/ChangeLog:8 > + Reviewed by NOBODY (OOPS!).
Please make everything up to the Reviewed by line consistent with the other ChangeLogs.
Brent Fulgham
Comment 28
2015-06-23 16:55:21 PDT
Comment on
attachment 255427
[details]
Patch This looks close! Need to correct the FeatureDefines issue I showed you (in person), and the ChangeLog format we discussed. Also, make sure to get rid of the root ChangeLog update (the first item above).
Matthew Daiter
Comment 29
2015-06-23 17:05:02 PDT
Created
attachment 255450
[details]
Patch
Brent Fulgham
Comment 30
2015-06-23 17:10:20 PDT
Comment on
attachment 255450
[details]
Patch Great! Let's get this integrated.
Build Bot
Comment 31
2015-06-23 17:57:55 PDT
Comment on
attachment 255450
[details]
Patch
Attachment 255450
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/5244379484651520
New failing tests: js/dom/global-constructors-attributes.html
Build Bot
Comment 32
2015-06-23 17:57:57 PDT
Created
attachment 255459
[details]
Archive of layout-test-results from ews103 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-mavericks Platform: Mac OS X 10.9.5
Build Bot
Comment 33
2015-06-23 18:02:56 PDT
Comment on
attachment 255450
[details]
Patch
Attachment 255450
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/5027738113015808
New failing tests: js/dom/global-constructors-attributes.html
Build Bot
Comment 34
2015-06-23 18:02:59 PDT
Created
attachment 255461
[details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Brent Fulgham
Comment 35
2015-06-24 10:02:56 PDT
The tests are still failing because you only updated the platform/mac test expectations. We need the same changes for mac-mavericks and mac-yosemite: bfulgham-macpro:platform bfulgham$ find . -name global-constructors-attributes-expected.txt ./efl/js/dom/global-constructors-attributes-expected.txt ./gtk/js/dom/global-constructors-attributes-expected.txt ./ios-sim-deprecated/js/dom/global-constructors-attributes-expected.txt ./mac/js/dom/global-constructors-attributes-expected.txt ./mac-mavericks/js/dom/global-constructors-attributes-expected.txt ./mac-yosemite/js/dom/global-constructors-attributes-expected.txt ./win/js/dom/global-constructors-attributes-expected.txt
Matthew Daiter
Comment 36
2015-06-24 10:23:04 PDT
Created
attachment 255490
[details]
Patch
Jon Lee
Comment 37
2015-06-24 10:37:10 PDT
Comment on
attachment 255490
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=255490&action=review
> LayoutTests/platform/mac-mavericks/js/dom/global-constructors-attributes-expected.txt:950 > +PASS Object.getOwnPropertyDescriptor(global, 'PerformanceTiming').configurable is true
This shouldn't be here.
> LayoutTests/platform/mac-mavericks/js/dom/global-constructors-attributes-expected.txt:2015 > +PASS Object.getOwnPropertyDescriptor(global, 'WebKitPlaybackTargetAvailabilityEvent').configurable is true
This shouldn't be here.
> LayoutTests/platform/mac-yosemite/js/dom/global-constructors-attributes-expected.txt:2015 > +PASS Object.getOwnPropertyDescriptor(global, 'WebKitPlaybackTargetAvailabilityEvent').configurable is true
This shouldn't be here.
Jon Lee
Comment 38
2015-06-24 10:55:51 PDT
Comment on
attachment 255490
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=255490&action=review
> Source/WebCore/ChangeLog:259 > +>>>>>>> .
r185915
Please remove
> Source/WebCore/Modules/mediastream/UserMediaClient.h:47 > + virtual void requestPermission(Ref<WebCore::UserMediaRequest>&&) = 0;
Why is the WebCore namespace prefix necessary?
> Source/WebKit2/ChangeLog:13 > +015-06-24 Brady Eidson <
beidson@apple.com
>
Looks like a bad merge.
> Source/WebKit2/WebProcess/WebPage/WebPage.h:468 > + WEBCORE_EXPORT UserMediaPermissionRequestManager& userMediaPermissionRequestManager() { return m_userMediaPermissionRequestManager; }
WebKit2 doesn't use this macro.
> Source/WebKit2/WebProcess/WebPage/WebPage.h:1084 > + WEBCORE_EXPORT void didReceiveUserMediaPermissionDecision(uint64_t userMediaID, bool allowed);
WebKit2 doesn't use this macro.
> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:172 > +- (void)denyOnlyThisRequest
These two methods should be wrapped in PLATFORM(IOS). The whole file is already wrapped in #if ENABLE(MEDIA_STREAM). Why are they needed inside each method?
> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:179 > +#endif
Since you're just trying to get everything to build in this patch, I'd just go with ASSERT_NOT_REACHED() until you get to the point where this needs to be implemented.
> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.mm:184 > + // FIXME:
https://bugs.webkit.org/show_bug.cgi?id=146245
Ditto.
> Tools/TestWebKitAPI/Tests/WebKit2/UserMedia.cpp:21 > +#if WK_HAVE_C_SPI
This is unfamiliar to me; where is it defined?
Brent Fulgham
Comment 39
2015-06-24 11:15:41 PDT
Comment on
attachment 255490
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=255490&action=review
>> Source/WebCore/ChangeLog:259 >> +>>>>>>> .
r185915
> > Please remove
Bad merge! Don't upload a patch with conflicting files!
>> Source/WebCore/Modules/mediastream/UserMediaClient.h:47 >> + virtual void requestPermission(Ref<WebCore::UserMediaRequest>&&) = 0; > > Why is the WebCore namespace prefix necessary?
I asked about this last night. Matt said he couldn't get it to build without the WebCore prefix. But perhaps he should try again.
>> Source/WebKit2/WebProcess/WebPage/WebPage.h:468 >> + WEBCORE_EXPORT UserMediaPermissionRequestManager& userMediaPermissionRequestManager() { return m_userMediaPermissionRequestManager; } > > WebKit2 doesn't use this macro.
This should be WK_EXPORT
>> Source/WebKit2/WebProcess/WebPage/WebPage.h:1084 >> + WEBCORE_EXPORT void didReceiveUserMediaPermissionDecision(uint64_t userMediaID, bool allowed); > > WebKit2 doesn't use this macro.
WK_EXPORT
>> Tools/TestWebKitAPI/Tests/WebKit2/UserMedia.cpp:21 >> +#if WK_HAVE_C_SPI > > This is unfamiliar to me; where is it defined?
Simon added this to Tools/TestWebKitAPI/config.h about a year ago. "Fix by removing some patterns from EXCLUDED_SOURCE_FILE_NAMES, and #ifdeffing source files instead. config.h defines WK_HAVE_C_SPI when the WebKit C SPI is available (i.e. Mac OS X), and most files use that. Some files with Mac-only functionality (ActionMenus, Downloads) use #if PLATFORM(MAC)."
Build Bot
Comment 40
2015-06-24 11:16:03 PDT
Comment on
attachment 255490
[details]
Patch
Attachment 255490
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.appspot.com/results/5686711187144704
New failing tests: js/dom/global-constructors-attributes.html
Build Bot
Comment 41
2015-06-24 11:16:08 PDT
Created
attachment 255494
[details]
Archive of layout-test-results from ews103 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-mavericks Platform: Mac OS X 10.9.5
Build Bot
Comment 42
2015-06-24 11:20:16 PDT
Comment on
attachment 255490
[details]
Patch
Attachment 255490
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.appspot.com/results/5104359255834624
New failing tests: js/dom/global-constructors-attributes.html
Build Bot
Comment 43
2015-06-24 11:20:19 PDT
Created
attachment 255496
[details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Matthew Daiter
Comment 44
2015-06-24 13:51:55 PDT
Created
attachment 255510
[details]
Patch
Matthew Daiter
Comment 45
2015-06-24 14:33:46 PDT
Created
attachment 255514
[details]
Patch
Brent Fulgham
Comment 46
2015-06-24 15:43:41 PDT
Comment on
attachment 255514
[details]
Patch r=me.
WebKit Commit Bot
Comment 47
2015-06-24 15:45:24 PDT
Comment on
attachment 255514
[details]
Patch Rejecting
attachment 255514
[details]
from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 255514, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit /Volumes/Data/EWS/WebKit/Source/WebKit/mac/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output:
http://webkit-queues.appspot.com/results/6610107144077312
Matthew Daiter
Comment 48
2015-06-24 16:05:32 PDT
Created
attachment 255524
[details]
Patch
Brent Fulgham
Comment 49
2015-06-24 16:31:05 PDT
Comment on
attachment 255524
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=255524&action=review
Missing some blank lines! r-
> Source/WebCore/ChangeLog:5 > + <
rdar://problem/21365829
>
You need a blank line here.
> Source/WebKit2/ChangeLog:5 > + <
rdar://problem/21365829
>
You need a blank line here.
> LayoutTests/ChangeLog:5 > + <
rdar://problem/21365829
>
You need a blank line here.
Matthew Daiter
Comment 50
2015-06-24 16:32:25 PDT
Created
attachment 255525
[details]
Patch
Brent Fulgham
Comment 51
2015-06-24 17:30:19 PDT
Comment on
attachment 255525
[details]
Patch r=me
WebKit Commit Bot
Comment 52
2015-06-24 18:20:28 PDT
Comment on
attachment 255525
[details]
Patch Rejecting
attachment 255525
[details]
from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 255525, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 185936 = c7267a958994970c7d7a5bf9039593f08f32d67b
r185939
= 2b93f4d8f70db54fdc664cfc978985a0df5442f2
r185940
= 88c1510b3a4f3993307f404b1893426951ac05de 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... Fast-forwarded master to refs/remotes/origin/master. Full output:
http://webkit-queues.appspot.com/results/6212078029242368
Brent Fulgham
Comment 53
2015-06-25 11:40:04 PDT
Committed
r185956
: <
http://trac.webkit.org/changeset/185956
>
Csaba Osztrogonác
Comment 54
2015-06-25 13:08:40 PDT
(In reply to
comment #53
)
> Committed
r185956
: <
http://trac.webkit.org/changeset/185956
>
It made all tests crash on the Apple Mac bots. :-/
WebKit Commit Bot
Comment 55
2015-06-25 13:50:21 PDT
Re-opened since this is blocked by
bug 146321
Brent Fulgham
Comment 56
2015-06-25 13:56:41 PDT
The crashes we saw on the main build system are surprising, given the clean EWS run. We might need a clean rebuild for this to work. I'm rolling it out for now to get the bots green again.
Brent Fulgham
Comment 57
2015-06-25 15:32:55 PDT
I'm seeing crashes with a local build, as well. I'm not sure why EWS claims that these tests are fine. Does EWS only run a subset of tests?
Brent Fulgham
Comment 58
2015-06-25 15:34:38 PDT
Matt, if you run tests locally in a Debug build, you see many crashes: [2250/36929] compositing/geometry/clipped-video-controller.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36429]) [2253/36929] accessibility/aria-hidden-hides-all-elements.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36404]) [2255/36929] compositing/images/direct-svg-image.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36448]) [2324/36929] contentfiltering/block-after-add-data-then-allow-unblock.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36411]) [3647/36929] css3/scroll-snap/add-element.html failed unexpectedly (test timed out, text diff) [3695/36929] css3/scroll-snap/dynamic-nested-elements.html failed unexpectedly (text diff) [3715/36929] contentfiltering/block-after-add-data-then-deny-unblock.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36533]) [3720/36929] css3/scroll-snap/scroll-snap-mandatory-resizeable.html failed unexpectedly (text diff) [3851/36929] compositing/regions/floated-region-with-transformed-child.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36443]) [3982/36929] css3/shapes/shape-outside/shape-image/shape-image-002.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36457]) [4505/36929] accessibility/input-auto-fill-button.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36527]) [4889/36929] css3/shapes/spec-examples/shape-outside-018.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36406]) [5167/36929] contentfiltering/block-after-add-data.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36551]) [5182/36929] css3/masking/mask-base64.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36434]) [5637/36929] css3/flexbox/flexitem.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36459]) [5637/36929] editing/input/password-echo-passnode2.html failed unexpectedly (com.apple.WebKit.WebContent.Development crashed [pid=36568]) Can you please replicate this on your system and figure out what's going on?
Matthew Daiter
Comment 59
2015-06-29 16:33:02 PDT
Created
attachment 255794
[details]
Patch
Matthew Daiter
Comment 60
2015-06-29 17:07:56 PDT
Created
attachment 255800
[details]
Patch
Matthew Daiter
Comment 61
2015-06-29 17:10:37 PDT
New patch today changed quite a bit. Needed to recompile, redo patch from scratch. Uploading new one.
Matthew Daiter
Comment 62
2015-06-30 10:18:28 PDT
Created
attachment 255827
[details]
Patch
Matthew Daiter
Comment 63
2015-06-30 10:19:05 PDT
Headers couldn't be found. Changed to private headers and re-uploaded.
Matthew Daiter
Comment 64
2015-06-30 12:13:20 PDT
Created
attachment 255837
[details]
Patch
Matthew Daiter
Comment 65
2015-06-30 12:14:41 PDT
Created
attachment 255838
[details]
Patch
Matthew Daiter
Comment 66
2015-06-30 12:37:53 PDT
Created
attachment 255842
[details]
Patch
Matthew Daiter
Comment 67
2015-06-30 15:05:46 PDT
Created
attachment 255862
[details]
Patch
Matthew Daiter
Comment 68
2015-06-30 17:10:25 PDT
Created
attachment 255880
[details]
Patch
Eric Carlson
Comment 69
2015-06-30 19:00:38 PDT
Comment on
attachment 255880
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=255880&action=review
> Source/WebCore/ChangeLog:18 > + (WebCore::MediaPlayerPrivateAVFoundation::load): Deleted. > + * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h: > + (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::load): Deleted. > + * platform/graphics/mac/MediaPlayerPrivateQTKit.h: > + (WebCore::MediaPlayerPrivateQTKit::load): Deleted.
It looks like these comments are incorrect.
> Source/WebCore/ChangeLog:24 > + (WebCore::refreshCaptureDeviceList): Deleted. > + (WebCore::AVCaptureDeviceManager::bestSourceForTypeAndConstraints): Deleted. > + (WebCore::AVCaptureDeviceManager::deviceDisconnected): Deleted.
Ditto.
> Source/WebCore/ChangeLog:28 > + (WebCore::AVMediaCaptureSource::~AVMediaCaptureSource): Deleted.
Ditto.
> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:157 > + virtual void load(MediaStreamPrivate*) override { }
Nit: "virtual" isn't necessary with "override"
> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:99 > + virtual void load(MediaStreamPrivate*) override { }
Ditto.
> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.h:90 > + virtual void load(MediaStreamPrivate*) override { }
Ditto.
> Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.h:49 > + virtual void validateRequestConstraints(PassRefPtr<MediaStreamCreationClient>, PassRefPtr<MediaConstraints> audioConstraints, PassRefPtr<MediaConstraints> videoConstraints) override; > + virtual void createMediaStream(PassRefPtr<MediaStreamCreationClient>, PassRefPtr<MediaConstraints> audioConstraints, PassRefPtr<MediaConstraints> videoConstraints) override;
Ditto.
> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.h:48 > + virtual void requestPermission(Ref<WebCore::UserMediaRequest>&&) override; > + virtual void cancelRequest(WebCore::UserMediaRequest&) override;
Ditto.
Matthew Daiter
Comment 70
2015-07-01 09:47:56 PDT
Comment on
attachment 255880
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=255880&action=review
>> Source/WebCore/ChangeLog:18 >> + (WebCore::MediaPlayerPrivateQTKit::load): Deleted. > > It looks like these comments are incorrect.
Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
>> Source/WebCore/ChangeLog:24 >> + (WebCore::AVCaptureDeviceManager::deviceDisconnected): Deleted. > > Ditto.
Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
>> Source/WebCore/ChangeLog:28 >> + (WebCore::AVMediaCaptureSource::~AVMediaCaptureSource): Deleted. > > Ditto.
Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
>> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:157 >> + virtual void load(MediaStreamPrivate*) override { } > > Nit: "virtual" isn't necessary with "override"
Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:99 >> + virtual void load(MediaStreamPrivate*) override { } > > Ditto.
Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
>> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.h:90 >> + virtual void load(MediaStreamPrivate*) override { } > > Ditto.
Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
>> Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.h:49 >> + virtual void createMediaStream(PassRefPtr<MediaStreamCreationClient>, PassRefPtr<MediaConstraints> audioConstraints, PassRefPtr<MediaConstraints> videoConstraints) override; > > Ditto.
Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
>> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.h:48 >> + virtual void cancelRequest(WebCore::UserMediaRequest&) override; > > Ditto.
Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
Matthew Daiter
Comment 71
2015-07-01 10:05:06 PDT
Comment on
attachment 255880
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=255880&action=review
Responses to comments.
>>> Source/WebCore/ChangeLog:18 >>> + (WebCore::MediaPlayerPrivateQTKit::load): Deleted. >> >> It looks like these comments are incorrect. > > Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
>>> Source/WebCore/ChangeLog:24 >>> + (WebCore::AVCaptureDeviceManager::deviceDisconnected): Deleted. >> >> Ditto. > > Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
>>> Source/WebCore/ChangeLog:28 >>> + (WebCore::AVMediaCaptureSource::~AVMediaCaptureSource): Deleted. >> >> Ditto. > > Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
>>> Source/WebCore/platform/graphics/avfoundation/MediaPlayerPrivateAVFoundation.h:157 >>> + virtual void load(MediaStreamPrivate*) override { } >> >> Nit: "virtual" isn't necessary with "override" > > Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
>>> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.h:99 >>> + virtual void load(MediaStreamPrivate*) override { } >> >> Ditto. > > Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
>>> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.h:90 >>> + virtual void load(MediaStreamPrivate*) override { } >> >> Ditto. > > Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
>>> Source/WebCore/platform/mediastream/mac/RealtimeMediaSourceCenterMac.h:49 >>> + virtual void createMediaStream(PassRefPtr<MediaStreamCreationClient>, PassRefPtr<MediaConstraints> audioConstraints, PassRefPtr<MediaConstraints> videoConstraints) override; >> >> Ditto. > > Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
>>> Source/WebKit/mac/WebCoreSupport/WebUserMediaClient.h:48 >>> + virtual void cancelRequest(WebCore::UserMediaRequest&) override; >> >> Ditto. > > Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
Okay. Just wanted to make sure the code I was committing synced with the rest of the coding style of the class I was modifying
Eric Carlson
Comment 72
2015-07-01 10:09:03 PDT
(In reply to
comment #71
)
> Comment on
attachment 255880
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=255880&action=review
> > Responses to comments. > > >>> Source/WebCore/ChangeLog:18 > >>> + (WebCore::MediaPlayerPrivateQTKit::load): Deleted. > >> > >> It looks like these comments are incorrect. > > > > Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen? >
It sounds like a bug in prepare-ChangeLog!
Matthew Daiter
Comment 73
2015-07-01 10:16:36 PDT
Created
attachment 255927
[details]
Patch
Brent Fulgham
Comment 74
2015-07-01 10:18:24 PDT
Comment on
attachment 255880
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=255880&action=review
>>>>> Source/WebCore/ChangeLog:18 >>>>> + (WebCore::MediaPlayerPrivateQTKit::load): Deleted. >>>> >>>> It looks like these comments are incorrect. >>> >>> Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen? >> >> Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen? > > It sounds like a bug in prepare-ChangeLog!
Prepare-ChangeLog isn't smart enough to remove lines from the ChangeLog once you've edited it. I think these lines were correct when you first ran prepare-ChangeLog, but after you modified your patch a few times they were no longer relevant.
>>>> Source/WebCore/ChangeLog:24 >>>> + (WebCore::AVCaptureDeviceManager::deviceDisconnected): Deleted. >>> >>> Ditto. >> >> Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen? > > Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
Ditto.
>>>> Source/WebCore/ChangeLog:28 >>>> + (WebCore::AVMediaCaptureSource::~AVMediaCaptureSource): Deleted. >>> >>> Ditto. >> >> Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen? > > Hm, I used prepare-ChangeLog to make these comments, and those were in the file when I started modifying them. Does this usually happen?
Ditto
Matthew Daiter
Comment 75
2015-07-01 10:24:31 PDT
Created
attachment 255930
[details]
Patch
Eric Carlson
Comment 76
2015-07-01 10:30:05 PDT
Comment on
attachment 255930
[details]
Patch This looks good, thanks for persevering! Marking r+, but I will wait to make sure the gtk bot is happy with it before marking it cq+
Brent Fulgham
Comment 77
2015-07-01 10:53:45 PDT
Comment on
attachment 255930
[details]
Patch gtk is happy as well. r+ for commit-queue.
WebKit Commit Bot
Comment 78
2015-07-01 11:45:26 PDT
Comment on
attachment 255930
[details]
Patch Clearing flags on attachment: 255930 Committed
r186182
: <
http://trac.webkit.org/changeset/186182
>
WebKit Commit Bot
Comment 79
2015-07-01 11:45:35 PDT
All reviewed patches have been landed. Closing bug.
Alex Christensen
Comment 80
2015-07-02 13:47:31 PDT
Build fix for Mavericks with Xcode 5.0.2 in
http://trac.webkit.org/changeset/186233
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug