Skip to content

Commit fe8ae06

Browse files
webkit-commit-queuewebkit-commit-queue
committed
Unreviewed, rolling out r241273.
https://bugs.webkit.org/show_bug.cgi?id=194579 This change is causing a flaky assertion failure crash in High Sierra Debug (Requested by ShawnRoberts on #webkit). Reverted changeset: "Stop using setDefersLoading from WebCore" https://bugs.webkit.org/show_bug.cgi?id=194315 https://trac.webkit.org/changeset/241273 Canonical link: https://commits.webkit.org/208996@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241350 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 2944bf1 commit fe8ae06

6 files changed

Lines changed: 63 additions & 35 deletions

File tree

‎Source/WebCore/ChangeLog‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
2019-02-13 Commit Queue <[email protected]>
2+
3+
Unreviewed, rolling out r241273.
4+
https://bugs.webkit.org/show_bug.cgi?id=194579
5+
6+
This change is causing a flaky assertion failure crash in High
7+
Sierra Debug (Requested by ShawnRoberts on #webkit).
8+
9+
Reverted changeset:
10+
11+
"Stop using setDefersLoading from WebCore"
12+
https://bugs.webkit.org/show_bug.cgi?id=194315
13+
https://trac.webkit.org/changeset/241273
14+
115
2019-02-12 Mark Lam <[email protected]>
216

317
Remove unnecessary null check in bindings.

‎Source/WebCore/loader/MediaResourceLoader.cpp‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,12 @@ void MediaResource::stop()
151151
m_resource = nullptr;
152152
}
153153

154+
void MediaResource::setDefersLoading(bool defersLoading)
155+
{
156+
if (m_resource)
157+
m_resource->setDefersLoading(defersLoading);
158+
}
159+
154160
void MediaResource::responseReceived(CachedResource& resource, const ResourceResponse& response, CompletionHandler<void()>&& completionHandler)
155161
{
156162
ASSERT_UNUSED(resource, &resource == m_resource);
@@ -172,12 +178,7 @@ void MediaResource::responseReceived(CachedResource& resource, const ResourceRes
172178

173179
m_didPassAccessControlCheck = m_resource->options().mode == FetchOptions::Mode::Cors;
174180
if (m_client)
175-
m_client->responseReceived(*this, response, [this, protectedThis = makeRef(*this), completionHandler = completionHandlerCaller.release()] (ShouldContinue shouldContinue) mutable {
176-
if (completionHandler)
177-
completionHandler();
178-
if (shouldContinue == ShouldContinue::No)
179-
stop();
180-
});
181+
m_client->responseReceived(*this, response);
181182

182183
m_loader->addResponseForTesting(response);
183184
}

‎Source/WebCore/loader/MediaResourceLoader.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ class MediaResource : public PlatformMediaResource, CachedRawResourceClient {
7575

7676
// PlatformMediaResource
7777
void stop() override;
78+
void setDefersLoading(bool) override;
7879
bool didPassAccessControlCheck() const override { return m_didPassAccessControlCheck; }
7980

8081
// CachedRawResourceClient

‎Source/WebCore/platform/graphics/PlatformMediaResourceLoader.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727

2828
#if ENABLE(VIDEO)
2929

30-
#include "PolicyChecker.h"
3130
#include <wtf/CompletionHandler.h>
3231
#include <wtf/Noncopyable.h>
3332
#include <wtf/RefCounted.h>
@@ -44,7 +43,7 @@ class PlatformMediaResourceClient {
4443
public:
4544
virtual ~PlatformMediaResourceClient() = default;
4645

47-
virtual void responseReceived(PlatformMediaResource&, const ResourceResponse&, CompletionHandler<void(ShouldContinue)>&& completionHandler) { completionHandler(ShouldContinue::Yes); }
46+
virtual void responseReceived(PlatformMediaResource&, const ResourceResponse&) { }
4847
virtual void redirectReceived(PlatformMediaResource&, ResourceRequest&& request, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&& completionHandler) { completionHandler(WTFMove(request)); }
4948
virtual bool shouldCacheResponse(PlatformMediaResource&, const ResourceResponse&) { return true; }
5049
virtual void dataSent(PlatformMediaResource&, unsigned long long, unsigned long long) { }
@@ -77,6 +76,7 @@ class PlatformMediaResource : public ThreadSafeRefCounted<PlatformMediaResource>
7776
PlatformMediaResource() = default;
7877
virtual ~PlatformMediaResource() = default;
7978
virtual void stop() { }
79+
virtual void setDefersLoading(bool) { }
8080
virtual bool didPassAccessControlCheck() const { return false; }
8181

8282
void setClient(std::unique_ptr<PlatformMediaResourceClient>&& client) { m_client = WTFMove(client); }

‎Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class CachedResourceStreamingClient final : public PlatformMediaResourceClient {
5050
void checkUpdateBlocksize(uint64_t bytesRead);
5151

5252
// PlatformMediaResourceClient virtual methods.
53-
void responseReceived(PlatformMediaResource&, const ResourceResponse&, CompletionHandler<void(ShouldContinue)>&&) override;
53+
void responseReceived(PlatformMediaResource&, const ResourceResponse&) override;
5454
void dataReceived(PlatformMediaResource&, const char*, int) override;
5555
void accessControlCheckFailed(PlatformMediaResource&, const ResourceError&) override;
5656
void loadFailed(PlatformMediaResource&, const ResourceError&) override;
@@ -683,7 +683,11 @@ static void webKitWebSrcNeedData(WebKitWebSrc* src)
683683
priv->paused = false;
684684

685685
GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
686-
priv->notifier->notify(MainThreadSourceNotification::NeedData, [protector] { });
686+
priv->notifier->notify(MainThreadSourceNotification::NeedData, [protector] {
687+
WebKitWebSrcPrivate* priv = protector->priv;
688+
if (priv->resource)
689+
priv->resource->setDefersLoading(false);
690+
});
687691
}
688692

689693
static void webKitWebSrcEnoughData(WebKitWebSrc* src)
@@ -700,7 +704,7 @@ static void webKitWebSrcEnoughData(WebKitWebSrc* src)
700704
priv->notifier->notify(MainThreadSourceNotification::EnoughData, [protector] {
701705
WebKitWebSrcPrivate* priv = protector->priv;
702706
if (priv->resource)
703-
priv->resource->stop();
707+
priv->resource->setDefersLoading(true);
704708
});
705709
}
706710

@@ -781,7 +785,7 @@ void CachedResourceStreamingClient::checkUpdateBlocksize(uint64_t bytesRead)
781785
}
782786
}
783787

784-
void CachedResourceStreamingClient::responseReceived(PlatformMediaResource&, const ResourceResponse& response, CompletionHandler<void(ShouldContinue)>&& completionHandler)
788+
void CachedResourceStreamingClient::responseReceived(PlatformMediaResource&, const ResourceResponse& response)
785789
{
786790
WebKitWebSrc* src = WEBKIT_WEB_SRC(m_src.get());
787791
WebKitWebSrcPrivate* priv = src->priv;
@@ -800,12 +804,12 @@ void CachedResourceStreamingClient::responseReceived(PlatformMediaResource&, con
800804
GST_ELEMENT_ERROR(src, RESOURCE, READ, ("Received %d HTTP error code", response.httpStatusCode()), (nullptr));
801805
gst_app_src_end_of_stream(priv->appsrc);
802806
webKitWebSrcStop(src);
803-
return completionHandler(ShouldContinue::No);
807+
return;
804808
}
805809

806810
if (priv->isSeeking) {
807811
GST_DEBUG_OBJECT(src, "Seek in progress, ignoring response");
808-
return completionHandler(ShouldContinue::Yes);
812+
return;
809813
}
810814

811815
if (priv->requestedOffset) {
@@ -818,7 +822,7 @@ void CachedResourceStreamingClient::responseReceived(PlatformMediaResource&, con
818822
GST_ELEMENT_ERROR(src, RESOURCE, READ, ("Received unexpected %d HTTP status code", response.httpStatusCode()), (nullptr));
819823
gst_app_src_end_of_stream(priv->appsrc);
820824
webKitWebSrcStop(src);
821-
return completionHandler(ShouldContinue::No);
825+
return;
822826
}
823827
}
824828

@@ -882,8 +886,6 @@ void CachedResourceStreamingClient::responseReceived(PlatformMediaResource&, con
882886
gst_element_post_message(GST_ELEMENT_CAST(src), gst_message_new_element(GST_OBJECT_CAST(src),
883887
gst_structure_copy(httpHeaders)));
884888
gst_pad_push_event(GST_BASE_SRC_PAD(priv->appsrc), gst_event_new_custom(GST_EVENT_CUSTOM_DOWNSTREAM_STICKY, httpHeaders));
885-
886-
completionHandler(ShouldContinue::Yes);
887889
}
888890

889891
void CachedResourceStreamingClient::dataReceived(PlatformMediaResource&, const char* data, int length)

‎Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm‎

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,11 @@ - (id)initWithSession:(WebCoreNSURLSession *)session identifier:(NSUInteger)iden
5353
- (void)_restart;
5454
- (void)_cancel;
5555
- (void)_finish;
56+
- (void)_setDefersLoading:(BOOL)defers;
5657
@property (assign) WebCoreNSURLSession * _Nullable session;
5758

5859
- (void)resource:(PlatformMediaResource&)resource sentBytes:(unsigned long long)bytesSent totalBytesToBeSent:(unsigned long long)totalBytesToBeSent;
59-
- (void)resource:(PlatformMediaResource&)resource receivedResponse:(const ResourceResponse&)response completionHandler:(CompletionHandler<void(ShouldContinue)>&&)completionHandler;
60+
- (void)resource:(PlatformMediaResource&)resource receivedResponse:(const ResourceResponse&)response;
6061
- (BOOL)resource:(PlatformMediaResource&)resource shouldCacheResponse:(const ResourceResponse&)response;
6162
- (void)resource:(PlatformMediaResource&)resource receivedData:(const char*)data length:(int)length;
6263
- (void)resource:(PlatformMediaResource&)resource receivedRedirect:(const ResourceResponse&)response request:(ResourceRequest&&)request completionHandler:(CompletionHandler<void(ResourceRequest&&)>&&)completionHandler;
@@ -381,7 +382,7 @@ - (BOOL)isKindOfClass:(Class)aClass
381382

382383
void clearTask();
383384

384-
void responseReceived(PlatformMediaResource&, const ResourceResponse&, CompletionHandler<void(ShouldContinue)>&&) override;
385+
void responseReceived(PlatformMediaResource&, const ResourceResponse&) override;
385386
void redirectReceived(PlatformMediaResource&, ResourceRequest&&, const ResourceResponse&, CompletionHandler<void(ResourceRequest&&)>&&) override;
386387
bool shouldCacheResponse(PlatformMediaResource&, const ResourceResponse&) override;
387388
void dataSent(PlatformMediaResource&, unsigned long long, unsigned long long) override;
@@ -410,13 +411,13 @@ - (BOOL)isKindOfClass:(Class)aClass
410411
[m_task resource:resource sentBytes:bytesSent totalBytesToBeSent:totalBytesToBeSent];
411412
}
412413

413-
void WebCoreNSURLSessionDataTaskClient::responseReceived(PlatformMediaResource& resource, const ResourceResponse& response, CompletionHandler<void(ShouldContinue)>&& completionHandler)
414+
void WebCoreNSURLSessionDataTaskClient::responseReceived(PlatformMediaResource& resource, const ResourceResponse& response)
414415
{
415416
LockHolder locker(m_taskLock);
416417
if (!m_task)
417-
return completionHandler(ShouldContinue::No);
418+
return;
418419

419-
[m_task resource:resource receivedResponse:response completionHandler:WTFMove(completionHandler)];
420+
[m_task resource:resource receivedResponse:response];
420421
}
421422

422423
bool WebCoreNSURLSessionDataTaskClient::shouldCacheResponse(PlatformMediaResource& resource, const ResourceResponse& response)
@@ -543,6 +544,13 @@ - (void)_finish
543544
[self resourceFinished:*_resource];
544545
}
545546

547+
- (void)_setDefersLoading:(BOOL)defers
548+
{
549+
ASSERT(isMainThread());
550+
if (_resource)
551+
_resource->setDefersLoading(defers);
552+
}
553+
546554
#pragma mark - NSURLSession API
547555
@synthesize session=_session;
548556
@synthesize taskIdentifier=_taskIdentifier;
@@ -627,37 +635,39 @@ - (void)resource:(PlatformMediaResource&)resource sentBytes:(unsigned long long)
627635
// No-op.
628636
}
629637

630-
- (void)resource:(PlatformMediaResource&)resource receivedResponse:(const ResourceResponse&)response completionHandler:(CompletionHandler<void(ShouldContinue)>&&)completionHandler
638+
- (void)resource:(PlatformMediaResource&)resource receivedResponse:(const ResourceResponse&)response
631639
{
632640
ASSERT(response.source() == ResourceResponse::Source::Network || response.source() == ResourceResponse::Source::DiskCache || response.source() == ResourceResponse::Source::DiskCacheAfterValidation || response.source() == ResourceResponse::Source::ServiceWorker);
633641
ASSERT_UNUSED(resource, &resource == _resource);
634642
ASSERT(isMainThread());
635643
[self.session task:self didReceiveResponseFromOrigin:SecurityOrigin::create(response.url())];
636644
[self.session task:self didReceiveCORSAccessCheckResult:resource.didPassAccessControlCheck()];
637645
self.countOfBytesExpectedToReceive = response.expectedContentLength();
646+
[self _setDefersLoading:YES];
638647
RetainPtr<NSURLResponse> strongResponse { response.nsURLResponse() };
639648
RetainPtr<WebCoreNSURLSessionDataTask> strongSelf { self };
640-
[self.session addDelegateOperation:[strongSelf, strongResponse, completionHandler = WTFMove(completionHandler)] () mutable {
649+
[self.session addDelegateOperation:[strongSelf, strongResponse] {
641650
strongSelf->_response = strongResponse.get();
642651

643652
id<NSURLSessionDataDelegate> dataDelegate = (id<NSURLSessionDataDelegate>)strongSelf.get().session.delegate;
644653
if (![dataDelegate respondsToSelector:@selector(URLSession:dataTask:didReceiveResponse:completionHandler:)]) {
645-
callOnMainThread([strongSelf, completionHandler = WTFMove(completionHandler)] () mutable {
646-
completionHandler(ShouldContinue::Yes);
654+
callOnMainThread([strongSelf] {
655+
[strongSelf _setDefersLoading:NO];
647656
});
648657
return;
649658
}
650659

651-
[dataDelegate URLSession:(NSURLSession *)strongSelf.get().session dataTask:(NSURLSessionDataTask *)strongSelf.get() didReceiveResponse:strongResponse.get() completionHandler:makeBlockPtr([strongSelf, completionHandler = WTFMove(completionHandler)] (NSURLSessionResponseDisposition disposition) mutable {
652-
callOnMainThread([strongSelf, disposition, completionHandler = WTFMove(completionHandler)] () mutable {
653-
if (disposition == NSURLSessionResponseCancel)
654-
completionHandler(ShouldContinue::No);
655-
else {
656-
ASSERT(disposition == NSURLSessionResponseAllow);
657-
completionHandler(ShouldContinue::Yes);
658-
}
660+
[dataDelegate URLSession:(NSURLSession *)strongSelf.get().session dataTask:(NSURLSessionDataTask *)strongSelf.get() didReceiveResponse:strongResponse.get() completionHandler:[strongSelf] (NSURLSessionResponseDisposition disposition) {
661+
if (disposition == NSURLSessionResponseCancel)
662+
[strongSelf cancel];
663+
else if (disposition == NSURLSessionResponseAllow)
664+
[strongSelf resume];
665+
else
666+
ASSERT_NOT_REACHED();
667+
callOnMainThread([strongSelf] {
668+
[strongSelf _setDefersLoading:NO];
659669
});
660-
}).get()];
670+
}];
661671
}];
662672
}
663673

0 commit comments

Comments
 (0)