WebContent crash when vending a media session for the touch bar.
<rdar://problem/30132707>
Created attachment 300466 [details] Patch
Created attachment 300469 [details] Patch
Comment on attachment 300469 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=300469&action=review r=me with comment. > Source/WebCore/html/HTMLMediaElement.cpp:586 > + if (auto document = page.mainFrame().document()) { We normally prefer earlier returns and auto*. auto* document = page.mainFrame().document(); if (!document) return false; String host = document->url().host(); return equalLettersIgnoringASCIICase(host, "netflix.com") || host.endsWithIgnoringASCIICase(".netflix.com");
Created attachment 300473 [details] Patch for landing
Comment on attachment 300473 [details] Patch for landing Rejecting attachment 300473 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 300473, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in Source/WebCore/ChangeLog contains OOPS!. Full output: http://webkit-queues.webkit.org/results/2994162
Comment on attachment 300473 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=300473&action=review > Source/WebCore/ChangeLog:10 > + The mainframe's document pointer may be null when tearing down a page upon navigation to a page that is in the > + page cache. If this triggers a GC sweep, we will attempt to reload touch bar media controls, which (as a part of Is the real bug here that work with side effects is being done during GC sweep? I don't think that it's an OK time for such work.
Comment on attachment 300473 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=300473&action=review >> Source/WebCore/ChangeLog:10 >> + page cache. If this triggers a GC sweep, we will attempt to reload touch bar media controls, which (as a part of > > Is the real bug here that work with side effects is being done during GC sweep? I don't think that it's an OK time for such work. Alexey is likely right that this work should not be done in the destructor but likely earlier (e.g. removed from document, ...). That being said, I still think having a null check here makes sense as there is never a guarantee that a frame has a document. It is also a safe change if needed for a branch. In addition to the null check though, we should find a better place to do this work than the destructor.
(In reply to comment #8) > Comment on attachment 300473 [details] > Patch for landing > > View in context: > https://bugs.webkit.org/attachment.cgi?id=300473&action=review > > >> Source/WebCore/ChangeLog:10 > >> + page cache. If this triggers a GC sweep, we will attempt to reload touch bar media controls, which (as a part of > > > > Is the real bug here that work with side effects is being done during GC sweep? I don't think that it's an OK time for such work. > > Alexey is likely right that this work should not be done in the destructor > but likely earlier (e.g. removed from document, ...). > > That being said, I still think having a null check here makes sense as there > is never a guarantee that a frame has a document. It is also a safe change > if needed for a branch. > In addition to the null check though, we should find a better place to do > this work than the destructor. See also: <rdar://problem/30340495>
Committed r211613: <http://trac.webkit.org/changeset/211613>