WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
178475
Add preliminary support for ServiceWorker Handle Fetch
https://bugs.webkit.org/show_bug.cgi?id=178475
Summary
Add preliminary support for ServiceWorker Handle Fetch
youenn fablet
Reported
2017-10-18 11:11:47 PDT
Let's implement Handle Fetch up to the creation of the fetch event
Attachments
Patch
(102.83 KB, patch)
2017-10-18 11:37 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Rebasing
(89.31 KB, patch)
2017-10-18 12:16 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
GTK fix try
(89.34 KB, patch)
2017-10-18 12:47 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
GTK fix try
(89.21 KB, patch)
2017-10-18 12:59 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(88.24 KB, patch)
2017-10-18 14:49 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(88.20 KB, patch)
2017-10-18 15:23 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(88.30 KB, patch)
2017-10-18 15:59 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch for landing
(88.52 KB, patch)
2017-10-18 18:29 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch
(89.01 KB, patch)
2017-10-19 13:37 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Show Obsolete
(8)
View All
Add attachment
proposed patch, testcase, etc.
youenn fablet
Comment 1
2017-10-18 11:37:50 PDT
Created
attachment 324142
[details]
Patch
youenn fablet
Comment 2
2017-10-18 12:16:05 PDT
Created
attachment 324146
[details]
Rebasing
youenn fablet
Comment 3
2017-10-18 12:47:27 PDT
Created
attachment 324150
[details]
GTK fix try
Chris Dumez
Comment 4
2017-10-18 12:56:06 PDT
Comment on
attachment 324146
[details]
Rebasing View in context:
https://bugs.webkit.org/attachment.cgi?id=324146&action=review
> Source/WebCore/ChangeLog:11 > + A script context is now storing its selected service worker identifer. This should be fully implemented later on.
typo: identifer
> Source/WebKit/ChangeLog:19 > + Future work should try to reduce the cases where the IPC danse is done unnecessarily.
typo: s/danse/dance/g
> Source/WebCore/loader/ResourceLoaderOptions.h:95 > +enum class ServiceWorkersMode {
Is ServiceWorkersMode better than ServiceWorkerMode?
> Source/WebCore/workers/service/ServiceWorkerProvider.h:28 > +#include <wtf/Function.h>
Why is this needed?
> Source/WebCore/workers/service/ServiceWorkerProvider.h:38 > +class CachedResource;
Why are those needed?
> Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.cpp:102 > +void WebSWServerConnection::didReceiveFetchResponse(uint64_t fetchIdentifier, const WebCore::ResourceResponse& response)
Unnecessary WebCore::
> Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h:57 > + void failedFetch(uint64_t fetchIdentifier);
naming is not very consistent with other fetch-related methods.
> Source/WebKit/StorageProcess/ServiceWorker/WebSWServerConnection.h:70 > +
One too many blank lines?
> Source/WebKit/StorageProcess/StorageProcess.messages.in:44 > + FailedFetch(uint64_t serverConnectionIdentifier, uint64_t fetchIdentifier)
Naming is not consistent with other fetch messages.
> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:222 > + WebProcess::singleton().webLoaderStrategy().scheduleLoadFromNetworkProcess(resourceLoader, request, WTFMove(trackingParameters), shouldClearReferrerOnHTTPSToHTTPRedirect, maximumBufferingTime);
This is confusing, it makes it look like we ask the service worker to handle to load, and then when it is not, we ask the networkProcess. We have to clarify this somehow. Lambda passed to methods are usually completion handlers. Maybe this could be a completion handler with an error parameter and we would only ask the network process if the error is set?
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:27 > +#include "ServiceWorkerClientFetch.h"
I think we usually put this include inside the #if ENABLE().
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:37 > +static bool shouldRedirectAsGET(const ResourceRequest& oldRequest, const WebCore::ResourceResponse& response, URL& newURL, bool crossOrigin)
Unnecessary WebCore:: I prefer originalRequest than oldRequest. The boolean should have a prefix, like isCrossOrigin. Seems like this method is generic and reusable. We may want to move it to the right header in WebCore. Also, isn't there already logic for this in WebCore somewhere?
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:62 > +static inline ResourceRequest computeNewRequest(const ResourceRequest& oldRequest, const WebCore::ResourceResponse& response)
originalRequest, necessary WebCore::
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:68 > + bool crossOrigin = !protocolHostAndPortAreEqual(oldRequest.url(), newURL);
isCrossOrigin
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:71 > + if (shouldRedirectAsGET(oldRequest, response, newURL, crossOrigin)) {
Should probably be merged with a && with the previous condition.
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:78 > + // FIXME: Properly handle Referer
Missing period at the end.
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h:48 > + ServiceWorkerClientFetch(Ref<WebCore::ResourceLoader>&& loader, uint64_t identifier, Ref<IPC::Connection>&& connection, WTF::Function<void(WebCore::ResourceLoader&, const WebCore::ResourceRequest&)>&& fallback)
We should probably move this to the cpp.
> Source/WebKit/WebProcess/Storage/ServiceWorkerContextManager.cpp:63 > + // Hard coding some fetches for testing purpose until we implement the creation of fetch event.
FIXME?
> Source/WebKit/WebProcess/Storage/ServiceWorkerContextManager.h:31 > +#include <WebCore/ExceptionOr.h>
Seems unused?
> Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp:82 > +// ASSERT(loader->options().serviceWorkerIdentifier);
Why is this commented?
> Source/WebKit/WebProcess/Storage/WebSWClientConnection.h:34 > +#include <WebCore/ResourceLoader.h>
Can this be a forward declaration?
> Source/WebKit/WebProcess/Storage/WebServiceWorkerProvider.cpp:61 > +static inline bool shouldHandleFetch(const WebSWClientConnection& connection, CachedResource& resource, const ResourceLoaderOptions& options)
Feels like this should maybe be a method on WebSWClientConnection.
> Source/WebKit/WebProcess/Storage/WebServiceWorkerProvider.h:50 > + HashMap<uint64_t, Ref<ServiceWorkerClientFetch>> m_ongoingFetches;
I don't think we should use "Fetches" since this is a verb. Maybe "FetchRequests"? Also, doesn't the spec use inflight instead of ongoing?
> Source/WebKit/WebProcess/Storage/WebServiceWorkerProvider.h:51 > + uint64_t m_nextOngoingFetchIdentifier { 0 };
Since we always need to increment it before using it, maybe m_lastFetchRequestIdentifier or m_previousFetchRequestIdentifier or m_currentFetchRequestIdentifier would be better.
> Source/WebKit/WebProcess/WebProcess.cpp:1653 > + if (!m_serviceWorkerManager)
Can this happen? It seems like this method used to always start a context and now it may fail. Why is it OK?
> Source/WebKit/WebProcess/WebProcess.messages.in:120 > +StartFetchInServiceWorker(uint64_t serverConnectionIdentifier, uint64_t fetchIdentifier, uint64_t serviceWorkerIdentifier, WebCore::ResourceRequest request, struct WebCore::FetchOptions options)
Indentation problem.
youenn fablet
Comment 5
2017-10-18 12:59:02 PDT
Created
attachment 324154
[details]
GTK fix try
youenn fablet
Comment 6
2017-10-18 14:49:05 PDT
Created
attachment 324169
[details]
Patch
youenn fablet
Comment 7
2017-10-18 15:04:39 PDT
(In reply to Chris Dumez from
comment #4
)
> Comment on
attachment 324146
[details]
> Rebasing
Thanks for the review. I updated according your comments. I removed redirection support for now. I moved to a completion handler taking an enum as well.
Chris Dumez
Comment 8
2017-10-18 15:20:31 PDT
Comment on
attachment 324169
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=324169&action=review
> Source/WebCore/dom/ScriptExecutionContext.h:306 > + uint64_t m_serviceWorkerIdentifier { 0 };
#if ENABLE(SERVICE_WORKER)
> Source/WebCore/workers/service/server/SWServer.h:43 > +class ResourceRequest;
Does not look needed.
> Source/WebCore/workers/service/server/SWServer.h:47 > +struct FetchOptions;
Does not look needed.
> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:220 > + WebServiceWorkerProvider::singleton().handleFetch(resourceLoader, resource, webPage ? webPage->sessionID() : PAL::SessionID::defaultSessionID(), [trackingParameters, shouldClearReferrerOnHTTPSToHTTPRedirect, maximumBufferingTime = maximumBufferingTime(resource), resourceLoader = makeRef(resourceLoader)] (ServiceWorkerClientFetchResult result) mutable {
How about we expose a 'canHandleFetch() method on WebServiceWorkerProvider? Then it's like: if (provider.canHandleFetch()) { provider.handleFetch() return; } scheduleLoadFromNetworkProcess(); Then I do not think we need the None/Unhandled enum value. We may not need an enum at all.
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:27 > +#include "ServiceWorkerClientFetch.h"
This is usually in the #if ENABLE() I believe.
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:53 > + ASSERT(response.httpStatusCode() <= 300 || response.httpStatusCode() >= 400 || response.httpStatusCode() == 304 || response.httpStatusCode() == 305 || response.httpStatusCode() == 306);
I do not think this should be an assertion since it can happen.
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h:39 > +enum class ServiceWorkerClientFetchResult { Done, Cancelled, None };
I think this should be moved to ServiceWorkerClientFetch class and simply called "Result". Also, I do not like the "None" result as I think it is unclear. How about "Succeeded / Cancelled / Unhandled"?
> Source/WebKit/WebProcess/Storage/ServiceWorkerContextManager.cpp:45 > + // FIXME: Provide a sensical session ID
Missing period at the end.
> Source/WebKit/WebProcess/Storage/ServiceWorkerContextManager.cpp:46 > +
No need for this extra blank line since the comment also applies to the next line.
> Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.cpp:37 > +#if ENABLE(SERVICE_WORKER)
Is this really needed?
youenn fablet
Comment 9
2017-10-18 15:23:14 PDT
Created
attachment 324174
[details]
Patch
youenn fablet
Comment 10
2017-10-18 15:26:56 PDT
> > Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:220 > > + WebServiceWorkerProvider::singleton().handleFetch(resourceLoader, resource, webPage ? webPage->sessionID() : PAL::SessionID::defaultSessionID(), [trackingParameters, shouldClearReferrerOnHTTPSToHTTPRedirect, maximumBufferingTime = maximumBufferingTime(resource), resourceLoader = makeRef(resourceLoader)] (ServiceWorkerClientFetchResult result) mutable { > > How about we expose a 'canHandleFetch() method on WebServiceWorkerProvider? > Then it's like: > if (provider.canHandleFetch()) { > provider.handleFetch() > return; > } > scheduleLoadFromNetworkProcess(); > > Then I do not think we need the None/Unhandled enum value. We may not need > an enum at all.
canHandleFetch might be asynchronous since we might need to go up to the service worker or storage process to discover we took the wrong path. I'll take care of the other comments.
Chris Dumez
Comment 11
2017-10-18 15:28:23 PDT
(In reply to youenn fablet from
comment #10
)
> > > > Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:220 > > > + WebServiceWorkerProvider::singleton().handleFetch(resourceLoader, resource, webPage ? webPage->sessionID() : PAL::SessionID::defaultSessionID(), [trackingParameters, shouldClearReferrerOnHTTPSToHTTPRedirect, maximumBufferingTime = maximumBufferingTime(resource), resourceLoader = makeRef(resourceLoader)] (ServiceWorkerClientFetchResult result) mutable { > > > > How about we expose a 'canHandleFetch() method on WebServiceWorkerProvider? > > Then it's like: > > if (provider.canHandleFetch()) { > > provider.handleFetch() > > return; > > } > > scheduleLoadFromNetworkProcess(); > > > > Then I do not think we need the None/Unhandled enum value. We may not need > > an enum at all. > > canHandleFetch might be asynchronous since we might need to go up to the > service worker or storage process to discover we took the wrong path. > > I'll take care of the other comments.
Then can we rename handleFetch() to something that indicates it may not handle it? e.g. tryHandleFetch() or handleFetchIfNecessary()
youenn fablet
Comment 12
2017-10-18 15:36:22 PDT
Comment on
attachment 324169
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=324169&action=review
>> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:27 >> +#include "ServiceWorkerClientFetch.h" > > This is usually in the #if ENABLE() I believe.
Style is usually to put both together. Since the header is wrapped in compile flag, this does not change any behavior.
>> Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.cpp:37 >> +#if ENABLE(SERVICE_WORKER) > > Is this really needed?
This is needed for build systems that do not generate this file yet. I guess we could add generation in CMakeLists.txt
Chris Dumez
Comment 13
2017-10-18 15:37:51 PDT
(In reply to youenn fablet from
comment #12
)
> Comment on
attachment 324169
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=324169&action=review
> > >> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:27 > >> +#include "ServiceWorkerClientFetch.h" > > > > This is usually in the #if ENABLE() I believe. > > Style is usually to put both together. > Since the header is wrapped in compile flag, this does not change any > behavior. > > >> Source/WebKit/WebProcess/Storage/WebToStorageProcessConnection.cpp:37 > >> +#if ENABLE(SERVICE_WORKER) > > > > Is this really needed? > > This is needed for build systems that do not generate this file yet. > I guess we could add generation in CMakeLists.txt
It looks like people do try to keep CMakeLists up to date when it comes to Service Workers.
youenn fablet
Comment 14
2017-10-18 15:45:01 PDT
> Then can we rename handleFetch() to something that indicates it may not > handle it? e.g. tryHandleFetch() or handleFetchIfNecessary()
I prefer keeping handleFetch. It refers to the
https://w3c.github.io/ServiceWorker/#handle-fetch
algorithm which can return null. As per fetch spec, if returning null, an http fetch will be triggered.
youenn fablet
Comment 15
2017-10-18 15:59:35 PDT
Created
attachment 324178
[details]
Patch
Chris Dumez
Comment 16
2017-10-18 16:31:56 PDT
Comment on
attachment 324178
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=324178&action=review
r=me with comments.
> Source/WebKit/WebProcess/Network/WebLoaderStrategy.h:80 > + void scheduleLoadFromNetworkProcess(WebCore::ResourceLoader&, const WebCore::ResourceRequest&, const WebResourceLoader::TrackingParameters&, bool shouldClearReferrerOnHTTPSToHTTPRedirect, Seconds);
I think we should provide the name of the last parameter as it is non-obvious.
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:53 > + if (!(response.httpStatusCode() <= 300 || response.httpStatusCode() >= 400 || response.httpStatusCode() == 304 || response.httpStatusCode() == 305 || response.httpStatusCode() == 306)) {
Can we add a isRedirect() method to ResourceResponseBase so this is reusable? Or a isRedirectResponse(response) function is some Webcore header?
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:67 > + if (auto callback = WTFMove(m_callback))
Why are we calling the completion callback with success here and not in didFinish()?
> Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h:64 > + RefPtr<WebCore::ResourceLoader> m_loader;
It looks like m_loader cannot currently be null. If so, it should probably be a Ref<> and we should get rid of all the null checks.
> Source/WebKit/WebProcess/Storage/WebServiceWorkerProvider.cpp:90 > + auto fetch = connection.startFetch(*this, loader, loader.identifier(), WTFMove(callback));
This is a risky pattern. It is common for networking code to fail synchronously, in which case, you'd try to remove the object from m_ongoingFetchTasks (and fail silently) and then add it to m_ongoingFetchTasks even though it is already finished.
> Source/WebKit/WebProcess/WebProcess.h:443 > + std::optional<ServiceWorkerContextManager> m_serviceWorkerManager;
Why optional and not unique_ptr?
youenn fablet
Comment 17
2017-10-18 17:24:37 PDT
(In reply to Chris Dumez from
comment #16
)
> Comment on
attachment 324178
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=324178&action=review
> > r=me with comments. > > > Source/WebKit/WebProcess/Network/WebLoaderStrategy.h:80 > > + void scheduleLoadFromNetworkProcess(WebCore::ResourceLoader&, const WebCore::ResourceRequest&, const WebResourceLoader::TrackingParameters&, bool shouldClearReferrerOnHTTPSToHTTPRedirect, Seconds); > > I think we should provide the name of the last parameter as it is > non-obvious.
OK
> > Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:53 > > + if (!(response.httpStatusCode() <= 300 || response.httpStatusCode() >= 400 || response.httpStatusCode() == 304 || response.httpStatusCode() == 305 || response.httpStatusCode() == 306)) { > > Can we add a isRedirect() method to ResourceResponseBase so this is > reusable? Or a isRedirectResponse(response) function is some Webcore header?
Let's add it when implementing redirections, since this check might be made differently.
> > Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.cpp:67 > > + if (auto callback = WTFMove(m_callback)) > > Why are we calling the completion callback with success here and not in > didFinish()?
We know that we will follow the SW path since we have a response that will go to the ResourceLoader. So SW handle fetch is returning a non null response, hence why calling the completion handler. SW may later on return an error if body has an issue.
> > Source/WebKit/WebProcess/Storage/ServiceWorkerClientFetch.h:64 > > + RefPtr<WebCore::ResourceLoader> m_loader; > > It looks like m_loader cannot currently be null. If so, it should probably > be a Ref<> and we should get rid of all the null checks.
OK, I put it as a RefPtr so that we can clean the loader in case of cancellation. But we are cleaning the whole fetch object so it can probably be a Ref.
> > Source/WebKit/WebProcess/Storage/WebServiceWorkerProvider.cpp:90 > > + auto fetch = connection.startFetch(*this, loader, loader.identifier(), WTFMove(callback)); > > This is a risky pattern. It is common for networking code to fail > synchronously, in which case, you'd try to remove the object from > m_ongoingFetchTasks (and fail silently) and then add it to > m_ongoingFetchTasks even though it is already finished.
I would guess in that case that fetch would be null, in which case it would be a RefPtr and we would not add it to the map.
> > Source/WebKit/WebProcess/WebProcess.h:443 > > + std::optional<ServiceWorkerContextManager> m_serviceWorkerManager; > > Why optional and not unique_ptr?
It removes the need for an additional memory allocation.
youenn fablet
Comment 18
2017-10-18 18:29:54 PDT
Created
attachment 324192
[details]
Patch for landing
WebKit Commit Bot
Comment 19
2017-10-18 19:26:04 PDT
Comment on
attachment 324192
[details]
Patch for landing Clearing flags on attachment: 324192 Committed
r223650
: <
https://trac.webkit.org/changeset/223650
>
WebKit Commit Bot
Comment 20
2017-10-18 19:26:06 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 21
2017-10-18 19:27:16 PDT
<
rdar://problem/35066424
>
Matt Lewis
Comment 22
2017-10-19 10:48:24 PDT
This revision cause multiple API Failures on all platforms. On Debug platforms it caused additional Assertion failures in API tests. Debug build:
https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK2%20%28Tests%29/builds/403
https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK2%20%28Tests%29/builds/403/steps/run-api-tests/logs/stdio
Release build:
https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/536
https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/536/steps/run-api-tests/logs/stdio
Talked to Chris and this doe not seem like an easy fix.
Chris Dumez
Comment 23
2017-10-19 10:58:51 PDT
(In reply to Matt Lewis from
comment #22
)
> This revision cause multiple API Failures on all platforms. On Debug > platforms it caused additional Assertion failures in API tests. > > Debug build: >
https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Debug%20WK2%20%28Tests%29/builds/403 >
https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Debug%20WK2%20%28Tests%29/builds/403/steps/run-api- > tests/logs/stdio > > Release build: >
https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/536 >
https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Release%20WK2%20%28Tests%29/builds/536/steps/run-api- > tests/logs/stdio > > Talked to Chris and this doe not seem like an easy fix.
Reverted in <
https://trac.webkit.org/changeset/223692
>.
youenn fablet
Comment 24
2017-10-19 13:37:52 PDT
Created
attachment 324278
[details]
Patch
Chris Dumez
Comment 25
2017-10-19 15:45:03 PDT
Comment on
attachment 324278
[details]
Patch Clearing flags on attachment: 324278 Committed
r223718
: <
https://trac.webkit.org/changeset/223718
>
Chris Dumez
Comment 26
2017-10-19 15:45:05 PDT
All reviewed patches have been landed. Closing bug.
Matt Lewis
Comment 27
2017-10-20 09:55:45 PDT
The new patch caused an api failure in WebKit.WebsiteDataStoreCustomPaths on all platforms. I was able to reproduce the issue: FAIL WebKit.WebsiteDataStoreCustomPaths /Volumes/Data/slave/highsierra-debug/build/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:219 Value of: [WKWebsiteDataStore _defaultDataStoreExists] Actual: true Expected: false
https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/builds/433/steps/run-api-tests/logs/stdio
https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/builds/433
Could this just need to have the expectations updated?
youenn fablet
Comment 28
2017-10-20 11:47:32 PDT
(In reply to Matt Lewis from
comment #27
)
> The new patch caused an api failure in WebKit.WebsiteDataStoreCustomPaths on > all platforms. > I was able to reproduce the issue: > > FAIL WebKit.WebsiteDataStoreCustomPaths > > /Volumes/Data/slave/highsierra-debug/build/Tools/TestWebKitAPI/Tests/ > WebKitCocoa/WebsiteDataStoreCustomPaths.mm:219 > Value of: [WKWebsiteDataStore _defaultDataStoreExists] > Actual: true > Expected: false > >
https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/builds/433/steps/run-api-tests/ > logs/stdio >
https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/builds/433 > > Could this just need to have the expectations updated?
Filed
https://bugs.webkit.org/show_bug.cgi?id=178596
. I will update the expectation and reopen the bug for further investigation.
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