Commit 0b792671 authored by Daniel Rubery's avatar Daniel Rubery Committed by Commit Bot

Observe WebContents for destruction in PasswordProtectionRequest

This CL observes WebContents and cancels the PasswordProtectionRequest
if the WebContents is destroyed. The destruction of WebContents is
responsible for at least this ZoomController bug, since I was manually
able to reproduce the callstack. The other bugs are added
speculatively, since they involve dereferencing the WebContents as
well.

Bug: 999030, 998218, 927884
Change-Id: I18fcd2ce8b8b6190bc8fed849813cca01b03d3f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1779579
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Reviewed-by: default avatarVarun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692327}
parent 35764892
...@@ -80,7 +80,8 @@ PasswordProtectionRequest::PasswordProtectionRequest( ...@@ -80,7 +80,8 @@ PasswordProtectionRequest::PasswordProtectionRequest(
bool password_field_exists, bool password_field_exists,
PasswordProtectionService* pps, PasswordProtectionService* pps,
int request_timeout_in_ms) int request_timeout_in_ms)
: web_contents_(web_contents), : content::WebContentsObserver(web_contents),
web_contents_(web_contents),
main_frame_url_(main_frame_url), main_frame_url_(main_frame_url),
password_form_action_(password_form_action), password_form_action_(password_form_action),
password_form_frame_url_(password_form_frame_url), password_form_frame_url_(password_form_frame_url),
...@@ -541,4 +542,8 @@ void PasswordProtectionRequest::HandleDeferredNavigations() { ...@@ -541,4 +542,8 @@ void PasswordProtectionRequest::HandleDeferredNavigations() {
throttles_.clear(); throttles_.clear();
} }
void PasswordProtectionRequest::WebContentsDestroyed() {
Cancel(/*timed_out=*/false);
}
} // namespace safe_browsing } // namespace safe_browsing
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "components/safe_browsing/password_protection/password_protection_service.h" #include "components/safe_browsing/password_protection/password_protection_service.h"
#include "components/safe_browsing/proto/csd.pb.h" #include "components/safe_browsing/proto/csd.pb.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/web_contents_observer.h"
#include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkBitmap.h"
class GURL; class GURL;
...@@ -50,10 +51,10 @@ using password_manager::metrics_util::PasswordType; ...@@ -50,10 +51,10 @@ using password_manager::metrics_util::PasswordType;
// (8) | UI | On receiving response, handle response and finish. // (8) | UI | On receiving response, handle response and finish.
// | | On request timeout, cancel request. // | | On request timeout, cancel request.
// | | On deletion of |password_protection_service_|, cancel request. // | | On deletion of |password_protection_service_|, cancel request.
class PasswordProtectionRequest class PasswordProtectionRequest : public base::RefCountedThreadSafe<
: public base::RefCountedThreadSafe< PasswordProtectionRequest,
PasswordProtectionRequest, content::BrowserThread::DeleteOnUIThread>,
content::BrowserThread::DeleteOnUIThread> { public content::WebContentsObserver {
public: public:
PasswordProtectionRequest(content::WebContents* web_contents, PasswordProtectionRequest(content::WebContents* web_contents,
const GURL& main_frame_url, const GURL& main_frame_url,
...@@ -122,6 +123,9 @@ class PasswordProtectionRequest ...@@ -122,6 +123,9 @@ class PasswordProtectionRequest
// Cancels navigation if there is modal warning showing, resumes it otherwise. // Cancels navigation if there is modal warning showing, resumes it otherwise.
void HandleDeferredNavigations(); void HandleDeferredNavigations();
// WebContentsObserver implementation
void WebContentsDestroyed() override;
protected: protected:
friend class base::RefCountedThreadSafe<PasswordProtectionRequest>; friend class base::RefCountedThreadSafe<PasswordProtectionRequest>;
...@@ -131,7 +135,7 @@ class PasswordProtectionRequest ...@@ -131,7 +135,7 @@ class PasswordProtectionRequest
friend class base::DeleteHelper<PasswordProtectionRequest>; friend class base::DeleteHelper<PasswordProtectionRequest>;
friend class PasswordProtectionServiceTest; friend class PasswordProtectionServiceTest;
friend class ChromePasswordProtectionServiceTest; friend class ChromePasswordProtectionServiceTest;
virtual ~PasswordProtectionRequest(); ~PasswordProtectionRequest() override;
// Start checking the whitelist. // Start checking the whitelist.
void CheckWhitelist(); void CheckWhitelist();
......
...@@ -1339,6 +1339,14 @@ TEST_P(PasswordProtectionServiceTest, TestRequestCancelNotOnTimeout) { ...@@ -1339,6 +1339,14 @@ TEST_P(PasswordProtectionServiceTest, TestRequestCancelNotOnTimeout) {
EXPECT_EQ(0U, GetNumberOfNavigationThrottles()); EXPECT_EQ(0U, GetNumberOfNavigationThrottles());
} }
TEST_P(PasswordProtectionServiceTest, TestWebContentsDestroyed) {
content::WebContents* web_contents = GetWebContents();
InitializeAndStartPasswordOnFocusRequest(
true /* match whitelist */, 10000 /* timeout in ms */, web_contents);
delete web_contents;
task_environment_.FastForwardUntilNoTasksRemain();
}
INSTANTIATE_TEST_SUITE_P(Regular, INSTANTIATE_TEST_SUITE_P(Regular,
PasswordProtectionServiceTest, PasswordProtectionServiceTest,
::testing::Values(false)); ::testing::Values(false));
......
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