Skip to content

Commit

Permalink
service worker: Remove debugging code for null http response info.
Browse files Browse the repository at this point in the history
The CHECK no longer fails after
https://crrev.com/815dc48c9d00b0142a2421de8f10b7cb9c5b34ab

BUG=485900

Review-Url: https://codereview.chromium.org/2655823004
Cr-Commit-Position: refs/heads/master@{#446305}
  • Loading branch information
mfalken authored and Commit bot committed Jan 26, 2017
1 parent 1e4463e commit 31562d3
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ ServiceWorkerContextRequestHandler::ServiceWorkerContextRequestHandler(
resource_type),
version_(provider_host_->running_hosted_version()) {
DCHECK(provider_host_->IsHostToRunningServiceWorker());
if (resource_type_ == RESOURCE_TYPE_SERVICE_WORKER)
version_->NotifyMainScriptRequestHandlerCreated();
}

ServiceWorkerContextRequestHandler::~ServiceWorkerContextRequestHandler() {
Expand Down Expand Up @@ -113,8 +111,6 @@ net::URLRequestJob* ServiceWorkerContextRequestHandler::MaybeCreateJob(
const bool is_main_script = resource_type_ == RESOURCE_TYPE_SERVICE_WORKER;
ServiceWorkerMetrics::RecordContextRequestHandlerStatus(
status, IsInstalled(version_.get()), is_main_script);
if (is_main_script)
version_->NotifyMainScriptJobCreated(status);
if (job)
return job;

Expand Down
27 changes: 3 additions & 24 deletions content/browser/service_worker/service_worker_script_cache_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,8 @@ void ServiceWorkerScriptCacheMap::NotifyStartedCaching(const GURL& url,
DCHECK(owner_->status() == ServiceWorkerVersion::NEW ||
owner_->status() == ServiceWorkerVersion::INSTALLING)
<< owner_->status();
if (!context_) {
if (IsMainScript(url)) {
main_script_start_status_ = StartStatus::NO_CONTEXT;
}
if (!context_)
return; // Our storage has been wiped via DeleteAndStartOver.
}
if (IsMainScript(url)) {
main_script_start_status_ = StartStatus::STARTED;
}
resource_map_[url] =
ServiceWorkerDatabase::ResourceRecord(resource_id, url, -1);
context_->storage()->StoreUncommittedResourceId(resource_id);
Expand All @@ -61,26 +54,16 @@ void ServiceWorkerScriptCacheMap::NotifyFinishedCaching(
DCHECK(owner_->status() == ServiceWorkerVersion::NEW ||
owner_->status() == ServiceWorkerVersion::INSTALLING ||
owner_->status() == ServiceWorkerVersion::REDUNDANT);
if (!context_) {
if (IsMainScript(url)) {
main_script_finish_status_ = FinishStatus::NO_CONTEXT;
}
if (!context_)
return; // Our storage has been wiped via DeleteAndStartOver.
}
if (net_error != net::OK) {
if (IsMainScript(url)) {
main_script_finish_status_ = FinishStatus::NET_ERROR;
}
context_->storage()->DoomUncommittedResource(LookupResourceId(url));
resource_map_.erase(url);
if (IsMainScript(url)) {
if (owner_->script_url() == url) {
main_script_status_ = net::URLRequestStatus::FromError(net_error);
main_script_status_message_ = status_message;
}
} else {
if (IsMainScript(url)) {
main_script_finish_status_ = FinishStatus::FINISHED;
}
resource_map_[url].size_bytes = size_bytes;
}
}
Expand Down Expand Up @@ -141,8 +124,4 @@ void ServiceWorkerScriptCacheMap::OnMetadataWritten(
callback.Run(result);
}

bool ServiceWorkerScriptCacheMap::IsMainScript(const GURL& url) {
return owner_->script_url() == url;
}

} // namespace content
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ class ServiceWorkerResponseMetadataWriter;
// for a particular version's implicit script resources.
class CONTENT_EXPORT ServiceWorkerScriptCacheMap {
public:
enum class StartStatus { UNINITIALIZED, NO_CONTEXT, STARTED };
enum class FinishStatus { UNINITIALIZED, NO_CONTEXT, NET_ERROR, FINISHED };

int64_t LookupResourceId(const GURL& url);

// Used during the initial run of a new version to build the map
Expand Down Expand Up @@ -87,15 +84,11 @@ class CONTENT_EXPORT ServiceWorkerScriptCacheMap {
const net::CompletionCallback& callback,
int result);

bool IsMainScript(const GURL& url);

ServiceWorkerVersion* owner_;
base::WeakPtr<ServiceWorkerContextCore> context_;
ResourceMap resource_map_;
net::URLRequestStatus main_script_status_;
std::string main_script_status_message_;
StartStatus main_script_start_status_ = StartStatus::UNINITIALIZED;
FinishStatus main_script_finish_status_ = FinishStatus::UNINITIALIZED;

base::WeakPtrFactory<ServiceWorkerScriptCacheMap> weak_factory_;

Expand Down
39 changes: 1 addition & 38 deletions content/browser/service_worker/service_worker_version.cc
Original file line number Diff line number Diff line change
Expand Up @@ -945,35 +945,7 @@ void ServiceWorkerVersion::OnDetached(EmbeddedWorkerStatus old_status) {
}

void ServiceWorkerVersion::OnScriptLoaded() {
// TODO(falken): Remove this CHECK once https://crbug.com/485900 is
// resolved.
if (!GetMainScriptHttpResponseInfo()) {
// Stick some information on the stack that may be useful in debugging.
Status status = status_;
char url[128];
base::strlcpy(url, script_url_.spec().c_str(), arraysize(url));
size_t script_map_size = script_cache_map_.size();
net::URLRequestStatus::Status main_script_status =
script_cache_map_.main_script_status().status();
ServiceWorkerScriptCacheMap::StartStatus start_status =
script_cache_map_.main_script_start_status_;
ServiceWorkerScriptCacheMap::FinishStatus finish_status =
script_cache_map_.main_script_finish_status_;
bool handler_created = main_script_request_handler_created_;
ServiceWorkerContextRequestHandler::CreateJobStatus job_created =
main_script_job_created_;

base::debug::Alias(&status);
base::debug::Alias(url);
base::debug::Alias(&script_map_size);
base::debug::Alias(&main_script_status);
base::debug::Alias(&start_status);
base::debug::Alias(&finish_status);
base::debug::Alias(&handler_created);
base::debug::Alias(&job_created);
CHECK(false);
}

DCHECK(GetMainScriptHttpResponseInfo());
if (IsInstalled(status()))
UMA_HISTOGRAM_BOOLEAN("ServiceWorker.ScriptLoadSuccess", true);
}
Expand Down Expand Up @@ -1130,15 +1102,6 @@ void ServiceWorkerVersion::OnSimpleEventFinished(
callback.Run(status);
}

void ServiceWorkerVersion::NotifyMainScriptRequestHandlerCreated() {
main_script_request_handler_created_ = true;
}

void ServiceWorkerVersion::NotifyMainScriptJobCreated(
ServiceWorkerContextRequestHandler::CreateJobStatus status) {
main_script_job_created_ = status;
}

ServiceWorkerVersion::NavigationPreloadSupportStatus
ServiceWorkerVersion::GetNavigationPreloadSupportStatus() const {
// The origin trial of Navigation Preload started from M57. And the worker
Expand Down
8 changes: 0 additions & 8 deletions content/browser/service_worker/service_worker_version.h
Original file line number Diff line number Diff line change
Expand Up @@ -433,10 +433,6 @@ class CONTENT_EXPORT ServiceWorkerVersion
ServiceWorkerStatusCode status,
base::Time dispatch_event_time);

void NotifyMainScriptRequestHandlerCreated();
void NotifyMainScriptJobCreated(
ServiceWorkerContextRequestHandler::CreateJobStatus status);

// Returns the Navigation Preload support status of the service worker.
// - Origin Trial: Have an effective token.
// Command line
Expand Down Expand Up @@ -847,10 +843,6 @@ class CONTENT_EXPORT ServiceWorkerVersion
// FinishStartWorker().
base::Optional<ServiceWorkerMetrics::EventType> start_worker_first_purpose_;

bool main_script_request_handler_created_ = false;
ServiceWorkerContextRequestHandler::CreateJobStatus main_script_job_created_ =
ServiceWorkerContextRequestHandler::CreateJobStatus::UNINITIALIZED;

base::WeakPtrFactory<ServiceWorkerVersion> weak_factory_;

DISALLOW_COPY_AND_ASSIGN(ServiceWorkerVersion);
Expand Down

0 comments on commit 31562d3

Please sign in to comment.