Provide a better way of injecting extra diagnostics in feedbacks

chrome/browser/extensions/api/feedback_private/feedback_browsertest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/api/feedback_private/feedback_private_api.h"
 #include "chrome/browser/extensions/component_loader.h"
@@ skipping 36 matching lines @@
   }
 
  protected:
   bool IsFeedbackAppAvailable() {
     return extensions::EventRouter::Get(browser()->profile())
         ->ExtensionHasEventListener(
             extension_misc::kFeedbackExtensionId,
             extensions::api::feedback_private::OnFeedbackRequested::kEventName);
   }
 
-  void StartFeedbackUI(FeedbackFlow flow) {
+  void StartFeedbackUI(FeedbackFlow flow,
+                       const std::string& extra_diagnostics) {
     base::Closure callback = base::Bind(&StopMessageLoopCallback);
     extensions::FeedbackPrivateGetStringsFunction::set_test_callback(&callback);
-    InvokeFeedbackUI(flow);
+    InvokeFeedbackUI(flow, extra_diagnostics);
     content::RunMessageLoop();
     extensions::FeedbackPrivateGetStringsFunction::set_test_callback(NULL);
   }
 
   void VerifyFeedbackAppLaunch() {
     AppWindow* window =
         PlatformAppBrowserTest::GetFirstAppWindowForBrowser(browser());
     ASSERT_TRUE(window);
     const Extension* feedback_app = window->GetExtension();
     ASSERT_TRUE(feedback_app);
     EXPECT_EQ(feedback_app->id(),
               std::string(extension_misc::kFeedbackExtensionId));
   }
 
  private:
-  void InvokeFeedbackUI(FeedbackFlow flow) {
+  void InvokeFeedbackUI(FeedbackFlow flow,
+                        const std::string& extra_diagnostics) {
     extensions::FeedbackPrivateAPI* api =
         extensions::FeedbackPrivateAPI::GetFactoryInstance()->Get(
             browser()->profile());
     api->RequestFeedbackForFlow("Test description", "Test tag",
-                                GURL("http://www.test.com"), flow);
+                                extra_diagnostics, GURL("http://www.test.com"),
+                                flow);
   }
 };
 
 IN_PROC_BROWSER_TEST_F(FeedbackTest, ShowFeedback) {
   WaitForExtensionViewsToLoad();
 
   ASSERT_TRUE(IsFeedbackAppAvailable());
-  StartFeedbackUI(FeedbackFlow::FEEDBACK_FLOW_REGULAR);
+  StartFeedbackUI(FeedbackFlow::FEEDBACK_FLOW_REGULAR,
+                  "" /* extra_diagnostics */);

[Peter Kasting, 2017/06/14 20:41:04]

Nit: Use std::string() in place of "" everywhere

[afakhry, 2017/06/14 22:10:18]

Done.

   VerifyFeedbackAppLaunch();
 }
 
 IN_PROC_BROWSER_TEST_F(FeedbackTest, ShowLoginFeedback) {
   WaitForExtensionViewsToLoad();
 
   ASSERT_TRUE(IsFeedbackAppAvailable());
-  StartFeedbackUI(FeedbackFlow::FEEDBACK_FLOW_LOGIN);
+  StartFeedbackUI(FeedbackFlow::FEEDBACK_FLOW_LOGIN,
+                  "" /* extra_diagnostics */);
   VerifyFeedbackAppLaunch();
 
   AppWindow* const window =
       PlatformAppBrowserTest::GetFirstAppWindowForBrowser(browser());
   ASSERT_TRUE(window);
   content::WebContents* const content = window->web_contents();
 
   bool bool_result = false;
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
       content,
       "domAutomationController.send("
         "$('page-url').hidden && $('attach-file-container').hidden && "
         "$('attach-file-note').hidden);",
       &bool_result));
   EXPECT_TRUE(bool_result);
 }
 
 // Tests that there's an option in the email drop down box with a value
 // 'anonymous_user'.
 IN_PROC_BROWSER_TEST_F(FeedbackTest, AnonymousUser) {
   WaitForExtensionViewsToLoad();
 
   ASSERT_TRUE(IsFeedbackAppAvailable());
-  StartFeedbackUI(FeedbackFlow::FEEDBACK_FLOW_REGULAR);
+  StartFeedbackUI(FeedbackFlow::FEEDBACK_FLOW_REGULAR,
+                  "" /* extra_diagnostics */);
   VerifyFeedbackAppLaunch();
 
   AppWindow* const window =
       PlatformAppBrowserTest::GetFirstAppWindowForBrowser(browser());
   ASSERT_TRUE(window);
   content::WebContents* const content = window->web_contents();
 
   bool bool_result = false;
   ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
       content,
       "domAutomationController.send("
       "  ((function() {"
       "      var options = $('user-email-drop-down').options;"
       "      for (var option in options) {"
       "        if (options[option].value == 'anonymous_user')"
       "          return true;"
       "      }"
       "      return false;"
       "    })()));",
       &bool_result));
 
   EXPECT_TRUE(bool_result);
 }
 
+// Makes sure that for feedbacks requested given extra diagnostics, the

[Peter Kasting, 2017/06/14 20:41:04]

Nit: Grammar; how about "Ensures that when extra diagnostics are provided with
feedback, they are injected properly in the system information."?

[afakhry, 2017/06/14 22:10:18]

Done.

+// diagnostics are injected properly in the system information.
+IN_PROC_BROWSER_TEST_F(FeedbackTest, ExtraDiagnostics) {
+  WaitForExtensionViewsToLoad();
+
+  ASSERT_TRUE(IsFeedbackAppAvailable());
+  StartFeedbackUI(FeedbackFlow::FEEDBACK_FLOW_REGULAR, "Some diagnostics");
+  VerifyFeedbackAppLaunch();
+
+  AppWindow* const window =
+      PlatformAppBrowserTest::GetFirstAppWindowForBrowser(browser());
+  ASSERT_TRUE(window);
+  content::WebContents* const content = window->web_contents();
+
+  bool bool_result = false;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractBool(
+      content,
+      "domAutomationController.send("
+      "  ((function() {"
+      "      var sysInfo = feedbackInfo.systemInformation;"
+      "      for (var info in sysInfo) {"
+      "        if (sysInfo[info].key == 'EXTRA_DIAGNOSTICS' &&"
+      "            sysInfo[info].value == 'Some diagnostics') {"
+      "          return true;"
+      "        }"
+      "      }"
+      "      return false;"
+      "    })()));",
+      &bool_result));
+
+  EXPECT_TRUE(bool_result);
+}
+
 }  // namespace extensions