Commit 1987a826 authored by xiang.long's avatar xiang.long Committed by Commit bot

ServiceWorker: Change worker script fetch error code.

Change worker script fetch error code in "Update" algorithm to align
with spec:
Non 2xx response will be rejected with NetworkError;
Redirected response will be rejected with SecurityError;
Reponse with Wrong MIME type will be rejected with SecurityError.

(1/3): https://codereview.chromium.org/546353003/
(2/3): this
(3/3): Layout test & Blink side cleanup

BUG=408421
TEST=http/tests/serviceworker

Review URL: https://codereview.chromium.org/567903002

Cr-Commit-Position: refs/heads/master@{#295940}
parent 71926f1a
...@@ -945,7 +945,8 @@ class UpdateJobTestHelper ...@@ -945,7 +945,8 @@ class UpdateJobTestHelper
int64 resource_id = storage()->NewResourceId(); int64 resource_id = storage()->NewResourceId();
version->script_cache_map()->NotifyStartedCaching(script, resource_id); version->script_cache_map()->NotifyStartedCaching(script, resource_id);
WriteStringResponse(storage(), resource_id, kMockScriptBody); WriteStringResponse(storage(), resource_id, kMockScriptBody);
version->script_cache_map()->NotifyFinishedCaching(script, true); version->script_cache_map()->NotifyFinishedCaching(
script, net::URLRequestStatus());
} else { } else {
// Spoof caching the script for the new version. // Spoof caching the script for the new version.
int64 resource_id = storage()->NewResourceId(); int64 resource_id = storage()->NewResourceId();
...@@ -954,7 +955,8 @@ class UpdateJobTestHelper ...@@ -954,7 +955,8 @@ class UpdateJobTestHelper
WriteStringResponse(storage(), resource_id, kMockScriptBody); WriteStringResponse(storage(), resource_id, kMockScriptBody);
else else
WriteStringResponse(storage(), resource_id, "mock_different_script"); WriteStringResponse(storage(), resource_id, "mock_different_script");
version->script_cache_map()->NotifyFinishedCaching(script, true); version->script_cache_map()->NotifyFinishedCaching(
script, net::URLRequestStatus());
} }
EmbeddedWorkerTestHelper::OnStartWorker( EmbeddedWorkerTestHelper::OnStartWorker(
embedded_worker_id, version_id, scope, script, pause_after_download); embedded_worker_id, version_id, scope, script, pause_after_download);
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "content/browser/service_worker/service_worker_registration.h" #include "content/browser/service_worker/service_worker_registration.h"
#include "content/browser/service_worker/service_worker_storage.h" #include "content/browser/service_worker/service_worker_storage.h"
#include "content/browser/service_worker/service_worker_utils.h" #include "content/browser/service_worker/service_worker_utils.h"
#include "net/base/net_errors.h"
namespace content { namespace content {
...@@ -333,14 +334,33 @@ void ServiceWorkerRegisterJob::UpdateAndContinue() { ...@@ -333,14 +334,33 @@ void ServiceWorkerRegisterJob::UpdateAndContinue() {
void ServiceWorkerRegisterJob::OnStartWorkerFinished( void ServiceWorkerRegisterJob::OnStartWorkerFinished(
ServiceWorkerStatusCode status) { ServiceWorkerStatusCode status) {
// "If serviceWorker fails to start up..." then reject the promise with an if (status == SERVICE_WORKER_OK) {
// error and abort. InstallAndContinue();
if (status != SERVICE_WORKER_OK) {
Complete(status);
return; return;
} }
InstallAndContinue(); // "If serviceWorker fails to start up..." then reject the promise with an
// error and abort. When there is a main script network error, the status will
// be updated to a more specific one.
const net::URLRequestStatus& main_script_status =
new_version()->script_cache_map()->main_script_status();
if (main_script_status.status() != net::URLRequestStatus::SUCCESS) {
switch (main_script_status.error()) {
case net::ERR_INSECURE_RESPONSE:
case net::ERR_UNSAFE_REDIRECT:
status = SERVICE_WORKER_ERROR_SECURITY;
break;
case net::ERR_ABORTED:
status = SERVICE_WORKER_ERROR_ABORT;
break;
case net::ERR_FAILED:
status = SERVICE_WORKER_ERROR_NETWORK;
break;
default:
NOTREACHED();
}
}
Complete(status);
} }
// This function corresponds to the spec's [[Install]] algorithm. // This function corresponds to the spec's [[Install]] algorithm.
......
...@@ -35,6 +35,14 @@ void GetServiceWorkerRegistrationStatusResponse( ...@@ -35,6 +35,14 @@ void GetServiceWorkerRegistrationStatusResponse(
*error_type = WebServiceWorkerError::ErrorTypeNotFound; *error_type = WebServiceWorkerError::ErrorTypeNotFound;
return; return;
case SERVICE_WORKER_ERROR_NETWORK:
*error_type = WebServiceWorkerError::ErrorTypeNetwork;
return;
case SERVICE_WORKER_ERROR_SECURITY:
*error_type = WebServiceWorkerError::ErrorTypeSecurity;
return;
case SERVICE_WORKER_ERROR_ABORT: case SERVICE_WORKER_ERROR_ABORT:
case SERVICE_WORKER_ERROR_IPC_FAILED: case SERVICE_WORKER_ERROR_IPC_FAILED:
case SERVICE_WORKER_ERROR_FAILED: case SERVICE_WORKER_ERROR_FAILED:
......
...@@ -16,8 +16,7 @@ ServiceWorkerScriptCacheMap::ServiceWorkerScriptCacheMap( ...@@ -16,8 +16,7 @@ ServiceWorkerScriptCacheMap::ServiceWorkerScriptCacheMap(
ServiceWorkerVersion* owner, ServiceWorkerVersion* owner,
base::WeakPtr<ServiceWorkerContextCore> context) base::WeakPtr<ServiceWorkerContextCore> context)
: owner_(owner), : owner_(owner),
context_(context), context_(context) {
has_error_(false) {
} }
ServiceWorkerScriptCacheMap::~ServiceWorkerScriptCacheMap() { ServiceWorkerScriptCacheMap::~ServiceWorkerScriptCacheMap() {
...@@ -40,14 +39,15 @@ void ServiceWorkerScriptCacheMap::NotifyStartedCaching( ...@@ -40,14 +39,15 @@ void ServiceWorkerScriptCacheMap::NotifyStartedCaching(
} }
void ServiceWorkerScriptCacheMap::NotifyFinishedCaching( void ServiceWorkerScriptCacheMap::NotifyFinishedCaching(
const GURL& url, bool success) { const GURL& url, const net::URLRequestStatus& status) {
DCHECK_NE(kInvalidServiceWorkerResponseId, Lookup(url)); DCHECK_NE(kInvalidServiceWorkerResponseId, Lookup(url));
DCHECK(owner_->status() == ServiceWorkerVersion::NEW || DCHECK(owner_->status() == ServiceWorkerVersion::NEW ||
owner_->status() == ServiceWorkerVersion::INSTALLING); owner_->status() == ServiceWorkerVersion::INSTALLING);
if (!success) { if (!status.is_success()) {
context_->storage()->DoomUncommittedResponse(Lookup(url)); context_->storage()->DoomUncommittedResponse(Lookup(url));
has_error_ = true;
resource_ids_.erase(url); resource_ids_.erase(url);
if (owner_->script_url() == url)
main_script_status_ = status;
} }
} }
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "content/browser/service_worker/service_worker_database.h" #include "content/browser/service_worker/service_worker_database.h"
#include "content/common/content_export.h" #include "content/common/content_export.h"
#include "net/url_request/url_request_status.h"
class GURL; class GURL;
...@@ -29,10 +30,10 @@ class CONTENT_EXPORT ServiceWorkerScriptCacheMap { ...@@ -29,10 +30,10 @@ class CONTENT_EXPORT ServiceWorkerScriptCacheMap {
// Used during the initial run of a new version to build the map // Used during the initial run of a new version to build the map
// of resources ids. // of resources ids.
void NotifyStartedCaching(const GURL& url, int64 resource_id); void NotifyStartedCaching(const GURL& url, int64 resource_id);
void NotifyFinishedCaching(const GURL& url, bool success); void NotifyFinishedCaching(const GURL& url,
const net::URLRequestStatus& status);
// Used to retrieve the results of the initial run of a new version. // Used to retrieve the results of the initial run of a new version.
bool HasError() const { return has_error_; }
void GetResources( void GetResources(
std::vector<ServiceWorkerDatabase::ResourceRecord>* resources); std::vector<ServiceWorkerDatabase::ResourceRecord>* resources);
...@@ -42,6 +43,10 @@ class CONTENT_EXPORT ServiceWorkerScriptCacheMap { ...@@ -42,6 +43,10 @@ class CONTENT_EXPORT ServiceWorkerScriptCacheMap {
size_t size() const { return resource_ids_.size(); } size_t size() const { return resource_ids_.size(); }
const net::URLRequestStatus& main_script_status() const {
return main_script_status_;
}
private: private:
typedef std::map<GURL, int64> ResourceIDMap; typedef std::map<GURL, int64> ResourceIDMap;
...@@ -55,7 +60,7 @@ class CONTENT_EXPORT ServiceWorkerScriptCacheMap { ...@@ -55,7 +60,7 @@ class CONTENT_EXPORT ServiceWorkerScriptCacheMap {
ServiceWorkerVersion* owner_; ServiceWorkerVersion* owner_;
base::WeakPtr<ServiceWorkerContextCore> context_; base::WeakPtr<ServiceWorkerContextCore> context_;
ResourceIDMap resource_ids_; ResourceIDMap resource_ids_;
bool has_error_; net::URLRequestStatus main_script_status_;
DISALLOW_COPY_AND_ASSIGN(ServiceWorkerScriptCacheMap); DISALLOW_COPY_AND_ASSIGN(ServiceWorkerScriptCacheMap);
}; };
......
...@@ -68,7 +68,8 @@ void ServiceWorkerWriteToCacheJob::Kill() { ...@@ -68,7 +68,8 @@ void ServiceWorkerWriteToCacheJob::Kill() {
net_request_.reset(); net_request_.reset();
if (did_notify_started_ && !did_notify_finished_) { if (did_notify_started_ && !did_notify_finished_) {
version_->script_cache_map()->NotifyFinishedCaching( version_->script_cache_map()->NotifyFinishedCaching(
url_, false); url_,
net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_ABORTED));
did_notify_finished_ = true; did_notify_finished_ = true;
} }
writer_.reset(); writer_.reset();
...@@ -128,7 +129,7 @@ bool ServiceWorkerWriteToCacheJob::ReadRawData( ...@@ -128,7 +129,7 @@ bool ServiceWorkerWriteToCacheJob::ReadRawData(
// No more data to process, the job is complete. // No more data to process, the job is complete.
io_buffer_ = NULL; io_buffer_ = NULL;
version_->script_cache_map()->NotifyFinishedCaching( version_->script_cache_map()->NotifyFinishedCaching(
url_, status.is_success()); url_, net::URLRequestStatus());
did_notify_finished_ = true; did_notify_finished_ = true;
return status.is_success(); return status.is_success();
} }
...@@ -279,7 +280,7 @@ void ServiceWorkerWriteToCacheJob::OnReceivedRedirect( ...@@ -279,7 +280,7 @@ void ServiceWorkerWriteToCacheJob::OnReceivedRedirect(
"ServiceWorkerWriteToCacheJob::OnReceivedRedirect"); "ServiceWorkerWriteToCacheJob::OnReceivedRedirect");
// Script resources can't redirect. // Script resources can't redirect.
AsyncNotifyDoneHelper(net::URLRequestStatus( AsyncNotifyDoneHelper(net::URLRequestStatus(
net::URLRequestStatus::FAILED, net::ERR_FAILED)); net::URLRequestStatus::FAILED, net::ERR_UNSAFE_REDIRECT));
} }
void ServiceWorkerWriteToCacheJob::OnAuthRequired( void ServiceWorkerWriteToCacheJob::OnAuthRequired(
...@@ -349,7 +350,7 @@ void ServiceWorkerWriteToCacheJob::OnResponseStarted( ...@@ -349,7 +350,7 @@ void ServiceWorkerWriteToCacheJob::OnResponseStarted(
mime_type != "text/javascript" && mime_type != "text/javascript" &&
mime_type != "application/javascript") { mime_type != "application/javascript") {
AsyncNotifyDoneHelper(net::URLRequestStatus( AsyncNotifyDoneHelper(net::URLRequestStatus(
net::URLRequestStatus::FAILED, net::ERR_FAILED)); net::URLRequestStatus::FAILED, net::ERR_INSECURE_RESPONSE));
return; return;
} }
} }
...@@ -380,8 +381,7 @@ void ServiceWorkerWriteToCacheJob::OnReadCompleted( ...@@ -380,8 +381,7 @@ void ServiceWorkerWriteToCacheJob::OnReadCompleted(
void ServiceWorkerWriteToCacheJob::AsyncNotifyDoneHelper( void ServiceWorkerWriteToCacheJob::AsyncNotifyDoneHelper(
const net::URLRequestStatus& status) { const net::URLRequestStatus& status) {
DCHECK(!status.is_io_pending()); DCHECK(!status.is_io_pending());
version_->script_cache_map()->NotifyFinishedCaching( version_->script_cache_map()->NotifyFinishedCaching(url_, status);
url_, status.is_success());
did_notify_finished_ = true; did_notify_finished_ = true;
SetStatus(status); SetStatus(status);
NotifyDone(status); NotifyDone(status);
......
...@@ -30,6 +30,10 @@ const char* ServiceWorkerStatusToString(ServiceWorkerStatusCode status) { ...@@ -30,6 +30,10 @@ const char* ServiceWorkerStatusToString(ServiceWorkerStatusCode status) {
return "ServiceWorker failed to activate"; return "ServiceWorker failed to activate";
case SERVICE_WORKER_ERROR_IPC_FAILED: case SERVICE_WORKER_ERROR_IPC_FAILED:
return "IPC connection was closed or IPC error has occured"; return "IPC connection was closed or IPC error has occured";
case SERVICE_WORKER_ERROR_NETWORK:
return "Operation failed by network issue";
case SERVICE_WORKER_ERROR_SECURITY:
return "Operation failed by security issue";
} }
NOTREACHED(); NOTREACHED();
return ""; return "";
......
...@@ -42,6 +42,12 @@ enum ServiceWorkerStatusCode { ...@@ -42,6 +42,12 @@ enum ServiceWorkerStatusCode {
// Sending an IPC to the worker failed (often due to child process is // Sending an IPC to the worker failed (often due to child process is
// terminated). // terminated).
SERVICE_WORKER_ERROR_IPC_FAILED, SERVICE_WORKER_ERROR_IPC_FAILED,
// Operation is failed by network issue.
SERVICE_WORKER_ERROR_NETWORK,
// Operation is failed by security issue.
SERVICE_WORKER_ERROR_SECURITY,
}; };
CONTENT_EXPORT const char* ServiceWorkerStatusToString( CONTENT_EXPORT const char* ServiceWorkerStatusToString(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment