Commit 638a51dd authored by mmenke's avatar mmenke Committed by Commit bot

Revert of Small fix for URLRequestJobs when entering suspending mode....

Revert of Small fix for URLRequestJobs when entering suspending mode. (patchset #3 id:40001 of https://codereview.chromium.org/1366203004/ )

Reason for revert:
Seeing odd failures that make no sense on android bots that may somehow be due to this CL...Trying a revert.  :(

Original issue's description:
> Small fix for URLRequestJobs when entering suspending mode.
>
> When entering suspend mode, there was a window of time between
> when the request was canceled, and when the NetworkDelegate was
> informed it was canceled.  This didn't exist for other cancellation
> paths, and if an embedder invoked certain callbacks in this period,
> thinking the request was still live, crashes could result.
>
> BUG=289715
>
> Committed: https://crrev.com/05608da225e05b4a20da3f4ca0cb9a5276fcf3d6
> Cr-Commit-Position: refs/heads/master@{#350938}

TBR=davidben@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=289715

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

Cr-Commit-Position: refs/heads/master@{#351170}
parent 866ec367
......@@ -280,18 +280,7 @@ HostPortPair URLRequestJob::GetSocketAddress() const {
}
void URLRequestJob::OnSuspend() {
// Most errors generated by the Job come as the result of the one current
// operation the job is waiting on returning an error. This event is unusual
// in that the Job may have another operation ongoing, or the Job may be idle
// and waiting on the next call.
//
// Need to cancel through the request to make sure everything is notified
// of the failure (Particularly that the NetworkDelegate, which the Job may be
// waiting on, is notified synchronously) and torn down correctly.
//
// TODO(mmenke): This should probably fail the request with
// NETWORK_IO_SUSPENDED instead.
request_->Cancel();
Kill();
}
void URLRequestJob::NotifyURLRequestDestroyed() {
......
......@@ -26,8 +26,6 @@
#include "base/memory/weak_ptr.h"
#include "base/message_loop/message_loop.h"
#include "base/path_service.h"
#include "base/power_monitor/power_monitor.h"
#include "base/power_monitor/power_monitor_source.h"
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/string_number_conversions.h"
......@@ -260,21 +258,6 @@ void TestLoadTimingNoHttpResponse(
}
#endif
// Test power monitor source that can simulate entering suspend mode. Can't use
// the one in base/ because it insists on bringing its own MessageLoop.
class TestPowerMonitorSource : public base::PowerMonitorSource {
public:
TestPowerMonitorSource() {}
~TestPowerMonitorSource() override {}
void Suspend() { ProcessPowerEvent(SUSPEND_EVENT); }
bool IsOnBatteryPowerImpl() override { return false; }
private:
DISALLOW_COPY_AND_ASSIGN(TestPowerMonitorSource);
};
// Do a case-insensitive search through |haystack| for |needle|.
bool ContainsString(const std::string& haystack, const char* needle) {
std::string::const_iterator it = std::search(
......@@ -3914,27 +3897,6 @@ TEST_F(URLRequestTestHTTP, UnexpectedServerAuthTest) {
}
}
// Tests that a request is cancelled while entering suspend mode.
TEST_F(URLRequestTestHTTP, CancelOnSuspend) {
TestPowerMonitorSource* power_monitor_source = new TestPowerMonitorSource();
base::PowerMonitor power_monitor(make_scoped_ptr(power_monitor_source));
ASSERT_TRUE(test_server_.Start());
TestDelegate d;
// Request that won't complete any time soon.
GURL url(test_server_.GetURL("slow?600"));
scoped_ptr<URLRequest> r(
default_context_.CreateRequest(url, DEFAULT_PRIORITY, &d));
r->Start();
power_monitor_source->Suspend();
// Wait for the suspend notification to cause the request to fail.
base::RunLoop().Run();
EXPECT_EQ(URLRequestStatus::CANCELED, r->status().status());
EXPECT_TRUE(d.request_failed());
EXPECT_EQ(1, default_network_delegate_.completed_requests());
}
TEST_F(URLRequestTestHTTP, GetTest_NoCache) {
ASSERT_TRUE(test_server_.Start());
......
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