Commit 52c6e53c authored by Minggang Wang's avatar Minggang Wang Committed by Commit Bot

Fix WorkerMainScriptLoaderTest.ResponseWithFailureThenOnComplete failure

Currently, when running the unit test
WorkerMainScriptLoaderTest.ResponseWithFailureThenOnComplete locally,
the result reports all tests passed:

[1/1] WorkerMainScriptLoaderTest.ResponseWithFailureThenOnComplete (12 ms)
SUCCESS: all tests passed.

but actually, there is an error reported:

Actual function call count doesn't match
EXPECT_CALL(*mock_observer, DidReceiveResponse(_, _, _, _, _))
Expected: to be called once
Actual: never called - unsatisfied and active

The root cause is that we must execute a forced GC in order to finalize
objects depending on the mock object before the test finished, and
the test begins to fail consistently after executing a GC in the dtor.
Meanwhile, we should call ResourceLoadObserver's DidReceiveResponsei()
method before stopping loading the main script by WorkerMainScriptLoader.

The WorkerMainScriptLoader class is designed to load the main script
for workers, which is pre-requested in browser process and passed to
renderer process through WorkerMainScriptLoadParams, and when the http
status code indicates a failure, we should notify the
|resource_load_observer_| by invoking its DidReceiveResponse() method,
thus the devtools could be aware of this response.

This patch implements that behavior to make the test runs successfully.

Change-Id: I03f062276abad2253b35ebfa4ed8b4f58eddab4f
Bug: 1132634
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2433631
Commit-Queue: Minggang Wang <minggang.wang@intel.com>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: default avatarYutaka Hirano <yhirano@chromium.org>
Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811929}
parent 2f103f00
...@@ -72,6 +72,11 @@ void WorkerMainScriptLoader::Start( ...@@ -72,6 +72,11 @@ void WorkerMainScriptLoader::Start(
resource_response_ = response.ToResourceResponse(); resource_response_ = response.ToResourceResponse();
NotifyResponseReceived(std::move(response_head)); NotifyResponseReceived(std::move(response_head));
resource_load_observer_->DidReceiveResponse(
initial_request_.InspectorId(), initial_request_, resource_response_,
/*resource=*/nullptr,
ResourceLoadObserver::ResponseSource::kNotFromMemoryCache);
if (resource_response_.IsHTTP() && if (resource_response_.IsHTTP() &&
!cors::IsOkStatus(resource_response_.HttpStatusCode())) { !cors::IsOkStatus(resource_response_.HttpStatusCode())) {
client_->OnFailedLoadingWorkerMainScript(); client_->OnFailedLoadingWorkerMainScript();
...@@ -85,10 +90,6 @@ void WorkerMainScriptLoader::Start( ...@@ -85,10 +90,6 @@ void WorkerMainScriptLoader::Start(
return; return;
} }
resource_load_observer_->DidReceiveResponse(
initial_request_.InspectorId(), initial_request_, resource_response_,
/*resource=*/nullptr,
ResourceLoadObserver::ResponseSource::kNotFromMemoryCache);
script_encoding_ = script_encoding_ =
resource_response_.TextEncodingName().IsEmpty() resource_response_.TextEncodingName().IsEmpty()
? UTF8Encoding() ? UTF8Encoding()
......
...@@ -40,6 +40,11 @@ class WorkerMainScriptLoaderTest : public testing::Test { ...@@ -40,6 +40,11 @@ class WorkerMainScriptLoaderTest : public testing::Test {
scoped_feature_list_.InitWithFeatureState( scoped_feature_list_.InitWithFeatureState(
blink::features::kPlzDedicatedWorker, true); blink::features::kPlzDedicatedWorker, true);
} }
~WorkerMainScriptLoaderTest() override {
// Forced GC in order to finalize objects depending on MockResourceObserver,
// see details https://crbug.com/1132634.
ThreadState::Current()->CollectAllGarbageForTesting();
}
protected: protected:
class TestPlatform final : public TestingPlatformSupport { class TestPlatform final : public TestingPlatformSupport {
......
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