Commit 869062d9 authored by Jeffrey Kardatzke's avatar Jeffrey Kardatzke Committed by Commit Bot

Add browsertest for complete process of sending feedback report

This adds a new browsertest which adds a hook into the feedback uploader
to catch it right before it does the actual report upload. This ensures
that clicking on the button to send a feedback report actually results
in the report getting queued to be sent. This covers a regression case
which is in crbug.com/954563.

Bug: 955039, 954563
Test: Browser tests pass, fail if prior regression was put back
Change-Id: I92762c1b6741ad057e78c8cafeb924f93995b249
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632534
Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666036}
parent 8214e825
...@@ -10,6 +10,8 @@ ...@@ -10,6 +10,8 @@
#include "chrome/browser/extensions/component_loader.h" #include "chrome/browser/extensions/component_loader.h"
#include "chrome/browser/extensions/extension_browsertest.h" #include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/feedback/feedback_dialog_utils.h" #include "chrome/browser/feedback/feedback_dialog_utils.h"
#include "chrome/browser/feedback/feedback_uploader_chrome.h"
#include "chrome/browser/feedback/feedback_uploader_factory_chrome.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser.h"
#include "chrome/common/extensions/extension_constants.h" #include "chrome/common/extensions/extension_constants.h"
...@@ -94,6 +96,18 @@ class FeedbackTest : public ExtensionBrowserTest { ...@@ -94,6 +96,18 @@ class FeedbackTest : public ExtensionBrowserTest {
} }
}; };
class TestFeedbackUploaderDelegate
: public feedback::FeedbackUploaderChrome::Delegate {
public:
explicit TestFeedbackUploaderDelegate(base::RunLoop* quit_on_dispatch)
: quit_on_dispatch_(quit_on_dispatch) {}
void OnStartDispatchingReport() override { quit_on_dispatch_->Quit(); }
private:
base::RunLoop* quit_on_dispatch_;
};
// Disabled for ASan due to flakiness on Mac ASan 64 Tests (1). // Disabled for ASan due to flakiness on Mac ASan 64 Tests (1).
// See crbug.com/757243. // See crbug.com/757243.
#if defined(ADDRESS_SANITIZER) #if defined(ADDRESS_SANITIZER)
...@@ -340,4 +354,49 @@ IN_PROC_BROWSER_TEST_F(FeedbackTest, GetTargetTabUrl) { ...@@ -340,4 +354,49 @@ IN_PROC_BROWSER_TEST_F(FeedbackTest, GetTargetTabUrl) {
DevToolsWindowTesting::CloseDevToolsWindowSync(devtools_window); DevToolsWindowTesting::CloseDevToolsWindowSync(devtools_window);
} }
} }
IN_PROC_BROWSER_TEST_F(FeedbackTest, SubmissionTest) {
WaitForExtensionViewsToLoad();
ASSERT_TRUE(IsFeedbackAppAvailable());
StartFeedbackUI(FeedbackFlow::FEEDBACK_FLOW_GOOGLEINTERNAL, std::string());
VerifyFeedbackAppLaunch();
AppWindow* const window =
PlatformAppBrowserTest::GetFirstAppWindowForBrowser(browser());
ASSERT_TRUE(window);
content::WebContents* const content = window->web_contents();
// Set a delegate for the uploader which will be invoked when the report
// normally would have been uploaded. We have it setup to then quit the
// RunLoop which will then allow us to terminate.
base::RunLoop run_loop;
TestFeedbackUploaderDelegate delegate(&run_loop);
feedback::FeedbackUploaderFactoryChrome::GetInstance()
->GetForBrowserContext(browser()->profile())
->set_feedback_uploader_delegate(&delegate);
// Click the send button.
bool bool_result = false;
ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
content,
"domAutomationController.send("
" ((function() {"
" if ($('send-report-button') != null) {"
" document.getElementById('send-report-button').click();"
" return true;"
" }"
" return false;"
" })()));",
&bool_result));
EXPECT_TRUE(bool_result);
// This will DCHECK if the JS private API call doesn't return a value, which
// is the main case we are concerned about.
run_loop.Run();
feedback::FeedbackUploaderFactoryChrome::GetInstance()
->GetForBrowserContext(browser()->profile())
->set_feedback_uploader_delegate(nullptr);
}
} // namespace extensions } // namespace extensions
...@@ -46,6 +46,9 @@ void FeedbackUploaderChrome::AccessTokenAvailable( ...@@ -46,6 +46,9 @@ void FeedbackUploaderChrome::AccessTokenAvailable(
} }
void FeedbackUploaderChrome::StartDispatchingReport() { void FeedbackUploaderChrome::StartDispatchingReport() {
if (delegate_)
delegate_->OnStartDispatchingReport();
access_token_.clear(); access_token_.clear();
// TODO(crbug.com/849591): Instead of getting the IdentityManager from the // TODO(crbug.com/849591): Instead of getting the IdentityManager from the
......
...@@ -28,6 +28,19 @@ class FeedbackUploaderChrome : public FeedbackUploader { ...@@ -28,6 +28,19 @@ class FeedbackUploaderChrome : public FeedbackUploader {
scoped_refptr<base::SingleThreadTaskRunner> task_runner); scoped_refptr<base::SingleThreadTaskRunner> task_runner);
~FeedbackUploaderChrome() override; ~FeedbackUploaderChrome() override;
class Delegate {
public:
// Notifies the delegate when we have started dispatching a feedback report.
virtual void OnStartDispatchingReport() = 0;
protected:
virtual ~Delegate() = default;
};
void set_feedback_uploader_delegate(Delegate* delegate) {
delegate_ = delegate;
}
private: private:
// feedback::FeedbackUploader: // feedback::FeedbackUploader:
void StartDispatchingReport() override; void StartDispatchingReport() override;
...@@ -41,6 +54,8 @@ class FeedbackUploaderChrome : public FeedbackUploader { ...@@ -41,6 +54,8 @@ class FeedbackUploaderChrome : public FeedbackUploader {
std::string access_token_; std::string access_token_;
Delegate* delegate_ = nullptr; // Not owned.
DISALLOW_COPY_AND_ASSIGN(FeedbackUploaderChrome); DISALLOW_COPY_AND_ASSIGN(FeedbackUploaderChrome);
}; };
......
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