Hats: Setup HatsDialog for Hats vNext

Modifies the HatsDialog to use trigger ID instead of site ID to load the
surveys. It also modifies loading the survey as a dialog instead of a
window with frame buttons, since the HTML survey already includes a
close button.

NOTE: This is a reupload of a previously abandoned CL. This approach may
have possible points of failure:
  - Survey may not load, leaving user with a blank window.
  - Allowed survey localizations may not match the device locale.
  - HaTS backend may choose to not load the survey.
Subsequent work will fix this issues. See go/cros-hats-next follow-up
work section.

Bug: 1087034
Test: Tested manually on linux chrome and atlas
Change-Id: I1453617284fcf1e30c8107643a763b7880fa3c73
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2386418
Commit-Queue: Malay Keshav <malaykeshav@chromium.org>
Reviewed-by: Xiyuan Slow <xiyuan@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803749}

chrome/browser/browser_resources.grd

@@ -312,7 +312,6 @@
         <include name="IDR_SLOW_CSS" file="resources\chromeos\slow.css" type="BINDATA" />
         <include name="IDR_SLOW_HTML" file="resources\chromeos\slow.html" type="BINDATA" />
         <include name="IDR_SLOW_JS" file="resources\chromeos\slow.js" type="BINDATA" />
-        <include name="IDR_HATS_HTML" file="resources\chromeos\hats\hats.html" flattenhtml="false" type="BINDATA" />
         <include name="IDR_DEMO_APP_MANIFEST" file="resources\chromeos\demo_app\manifest.json" type="BINDATA" />
         <include name="IDR_WALLPAPERMANAGER_MANIFEST" file="resources\chromeos\wallpaper_manager\manifest.json" type="BINDATA" />
         <include name="IDR_FIRST_RUN_DIALOG_MANIFEST" file="resources\chromeos\first_run\app/manifest.json" type="BINDATA" />

chrome/browser/chromeos/hats/hats_dialog.cc

@@ -9,6 +9,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/task/post_task.h"
 #include "base/task/thread_pool.h"
+#include "chrome/browser/chromeos/hats/hats_finch_helper.h"
 #include "chrome/browser/profiles/profile_destroyer.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_dialogs.h"
@@ -33,22 +34,17 @@ namespace chromeos {
 namespace {
 
 // Default width/height ratio of screen size.
-const int kDefaultWidth = 400;
-const int kDefaultHeight = 420;
-// Site ID for HaTS survey.
-constexpr char kRegularSiteID[] = "cs5lsagwwbho7l5cbbdniso22e";
-constexpr char kGooglerSiteID[] = "z56p2hjy7pegxh3gmmur4qlwha";
-
-constexpr char kScriptSrcReplacementToken[] = "$SCRIPT_SRC";
-constexpr char kDoneButtonLabelReplacementToken[] = "$DONE_BUTTON_LABEL";
-// Base URL to fetch the google consumer survey script.
-constexpr char kBaseFormatUrl[] =
-    "https://www.google.com/insights/consumersurveys/"
-    "async_survey?site=%s&force_https=1&sc=%s";
+const int kDefaultWidth = 340;
+const int kDefaultHeight = 260;
+
+constexpr char kCrOSHaTSURL[] =
+    "https://storage.googleapis.com/chromeos-hats-web-stable/index.html";
+
 // Keyword used to join the separate device info elements into a single string
 // to be used as site context.
-const char kDeviceInfoStopKeyword[] = "STOP";
+const char kDeviceInfoStopKeyword[] = "&";
 const char kDefaultProfileLocale[] = "en-US";
+
 enum class DeviceInfoKey : unsigned int {
   BROWSER = 0,
   PLATFORM,
@@ -56,25 +52,6 @@ enum class DeviceInfoKey : unsigned int {
   LOCALE,
 };
 
-// Returns the local HaTS HTML file as a string with the correct Hats script
-// URL.
-std::string LoadLocalHtmlAsString(const std::string& site_id,
-                                  const std::string& site_context) {
-  std::string html_data =
-      ui::ResourceBundle::GetSharedInstance().LoadDataResourceString(
-          IDR_HATS_HTML);
-
-  size_t pos = html_data.find(kScriptSrcReplacementToken);
-  html_data.replace(pos, strlen(kScriptSrcReplacementToken),
-                    base::StringPrintf(kBaseFormatUrl, site_id.c_str(),
-                                       site_context.c_str()));
-
-  pos = html_data.find(kDoneButtonLabelReplacementToken);
-  html_data.replace(pos, strlen(kDoneButtonLabelReplacementToken),
-                    l10n_util::GetStringUTF8(IDS_HATS_DONE_BUTTON_LABEL));
-  return html_data;
-}
-
 // Maps the given DeviceInfoKey |key| enum to the corresponding string value
 // that can be used as a key when creating a URL parameter.
 const std::string KeyEnumToString(DeviceInfoKey key) {
@@ -96,40 +73,28 @@ const std::string KeyEnumToString(DeviceInfoKey key) {
 // Must be run on a blocking thread pool.
 // Gathers the browser version info, firmware info and platform info and returns
 // them in a single encoded string, the format of which is defined below.
-// Currently the format is "<key><value>STOP<key><value>STOP<key><value>" where
-// 'STOP' is used as a token to identify the end of a key value pair. This is
-// done since GCS only allows the use of alphanumeric characters to be passed as
-// a site context.
+// Currently the format is "<key>=<value>&<key>=<value>&<key>=<value>".
 std::string GetFormattedSiteContext(const std::string& user_locale,
                                     base::StringPiece join_keyword) {
   std::vector<std::string> pairs;
-  pairs.push_back(KeyEnumToString(DeviceInfoKey::BROWSER) +
+  pairs.push_back(KeyEnumToString(DeviceInfoKey::BROWSER) + "=" +
                   version_info::GetVersionNumber());
 
-  pairs.push_back(KeyEnumToString(DeviceInfoKey::PLATFORM) +
+  pairs.push_back(KeyEnumToString(DeviceInfoKey::PLATFORM) + "=" +
                   version_loader::GetVersion(version_loader::VERSION_FULL));
 
-  pairs.push_back(KeyEnumToString(DeviceInfoKey::FIRMWARE) +
+  pairs.push_back(KeyEnumToString(DeviceInfoKey::FIRMWARE) + "=" +
                   version_loader::GetFirmware());
 
-  pairs.push_back(KeyEnumToString(DeviceInfoKey::LOCALE) + user_locale);
+  pairs.push_back(KeyEnumToString(DeviceInfoKey::LOCALE) + "=" + user_locale);
 
   return base::JoinString(pairs, join_keyword);
 }
 
-// Determine which HaTS survey to show the user.
-const std::string GetSiteID(bool is_google_account) {
-  if (is_google_account) {
-    return kGooglerSiteID;
-  } else {
-    return kRegularSiteID;
-  }
-}
-
 }  // namespace
 
 // static
-void HatsDialog::CreateAndShow(bool is_google_account) {
+std::unique_ptr<HatsDialog> HatsDialog::CreateAndShow() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   Profile* profile = ProfileManager::GetActiveUserProfile();
@@ -139,44 +104,39 @@ void HatsDialog::CreateAndShow(bool is_google_account) {
   if (!user_locale.length())
     user_locale = kDefaultProfileLocale;
 
+  std::unique_ptr<HatsDialog> hats_dialog(
+      new HatsDialog(HatsFinchHelper::GetTriggerID(), profile));
+
+  // Raw pointer is used here since the dialog is owned by the hats
+  // notification controller which lives until the end of the user session. The
+  // dialog will always be closed before that time instant.
   base::ThreadPool::PostTaskAndReplyWithResult(
       FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
       base::BindOnce(&GetFormattedSiteContext, user_locale,
                      kDeviceInfoStopKeyword),
-      base::BindOnce(&HatsDialog::Show, is_google_account));
+      base::BindOnce(&HatsDialog::Show, base::Unretained(hats_dialog.get())));
+
+  return hats_dialog;
 }
 
-// static
-void HatsDialog::Show(bool is_google_account, const std::string& site_context) {
+void HatsDialog::Show(const std::string& site_context) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  // Load and set the html data that needs to be displayed in the dialog.
-  std::string site_id = GetSiteID(is_google_account);
-  std::string html_data = LoadLocalHtmlAsString(site_id, site_context);
+  // Link the trigger ID to fetch the correct survey.
+  url_ = std::string(kCrOSHaTSURL) + "?" + site_context +
+         "&trigger=" + trigger_id_;
 
-  Profile* active_profile = ProfileManager::GetActiveUserProfile();
-  Profile* profile_to_show = active_profile->GetOffTheRecordProfile(
-      Profile::OTRProfileID::CreateUnique("ChromeOS::HatsDialog"));
-
-  // Self deleting.
-  // Users of non-primary OTR profiles should destroy it when it's not needed
-  // any more.
-  auto* hats_dialog = new HatsDialog(html_data,
-                                     /* otr_profile= */ profile_to_show);
-  chrome::ShowWebDialog(nullptr, profile_to_show, hats_dialog);
+  chrome::ShowWebDialog(nullptr, ProfileManager::GetActiveUserProfile(), this);
 }
 
-HatsDialog::HatsDialog(const std::string& html_data, Profile* otr_profile)
-    : html_data_(html_data), otr_profile_(otr_profile) {
+HatsDialog::HatsDialog(const std::string& trigger_id, Profile* user_profile)
+    : trigger_id_(trigger_id), user_profile_(user_profile) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK(otr_profile_->IsOffTheRecord());
-  DCHECK(!otr_profile_->IsPrimaryOTRProfile());
 }
 
 HatsDialog::~HatsDialog() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK(otr_profile_);
-  ProfileDestroyer::DestroyProfileWhenAppropriate(otr_profile_);
+  DCHECK(user_profile_);
 }
 
 ui::ModalType HatsDialog::GetDialogModalType() const {
@@ -188,7 +148,7 @@ base::string16 HatsDialog::GetDialogTitle() const {
 }
 
 GURL HatsDialog::GetDialogContentURL() const {
-  return GURL("data:text/html;charset=utf-8," + html_data_);
+  return GURL(url_);
 }
 
 void HatsDialog::GetWebUIMessageHandlers(
@@ -206,9 +166,7 @@ std::string HatsDialog::GetDialogArgs() const {
   return std::string();
 }
 
-void HatsDialog::OnDialogClosed(const std::string& json_retval) {
-  delete this;
-}
+void HatsDialog::OnDialogClosed(const std::string& json_retval) {}
 
 void HatsDialog::OnCloseContents(WebContents* source, bool* out_close_dialog) {
   *out_close_dialog = true;
@@ -218,10 +176,18 @@ bool HatsDialog::ShouldShowDialogTitle() const {
   return false;
 }
 
+bool HatsDialog::ShouldShowCloseButton() const {
+  return false;
+}
+
 bool HatsDialog::HandleContextMenu(content::RenderFrameHost* render_frame_host,
                                    const content::ContextMenuParams& params) {
-  // Disable context menu.
+  // Disable context menu
   return true;
 }
 
+ui::WebDialogDelegate::FrameKind HatsDialog::GetWebDialogFrameKind() const {
+  return ui::WebDialogDelegate::FrameKind::kDialog;
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/hats/hats_dialog.h

@@ -16,19 +16,18 @@ namespace chromeos {
 
 // Happiness tracking survey dialog. Sometimes appears after login to ask the
 // user how satisfied they are with their Chromebook.
-// This class lives on the UI thread and is self deleting.
+// This class lives on the UI thread.
 class HatsDialog : public ui::WebDialogDelegate {
  public:
   // Creates an instance of HatsDialog and posts a task to load all the relevant
   // device info before displaying the dialog.
-  static void CreateAndShow(bool is_google_account);
+  static std::unique_ptr<HatsDialog> CreateAndShow();
+  ~HatsDialog() override;
 
  private:
-  static void Show(bool is_google_account, const std::string& site_context);
+  void Show(const std::string& site_context);
 
-  // Use CreateAndShow() above.
-  explicit HatsDialog(const std::string& html_data, Profile* otr_profile);
-  ~HatsDialog() override;
+  explicit HatsDialog(const std::string& trigger_id, Profile* user_profile);
 
   // ui::WebDialogDelegate implementation.
   ui::ModalType GetDialogModalType() const override;
@@ -39,16 +38,18 @@ class HatsDialog : public ui::WebDialogDelegate {
   void GetDialogSize(gfx::Size* size) const override;
   bool CanResizeDialog() const override;
   std::string GetDialogArgs() const override;
-  // NOTE: This function deletes this object at the end.
   void OnDialogClosed(const std::string& json_retval) override;
   void OnCloseContents(content::WebContents* source,
                        bool* out_close_dialog) override;
   bool ShouldShowDialogTitle() const override;
+  bool ShouldShowCloseButton() const override;
   bool HandleContextMenu(content::RenderFrameHost* render_frame_host,
                          const content::ContextMenuParams& params) override;
+  ui::WebDialogDelegate::FrameKind GetWebDialogFrameKind() const override;
 
-  const std::string html_data_;
-  Profile* otr_profile_;
+  const std::string trigger_id_;
+  std::string url_;
+  Profile* user_profile_;
 
   DISALLOW_COPY_AND_ASSIGN(HatsDialog);
 };

chrome/browser/chromeos/hats/hats_finch_helper.cc

@@ -25,6 +25,14 @@ const char HatsFinchHelper::kSurveyStartDateMsParam[] = "survey_start_date_ms";
 const char HatsFinchHelper::kResetSurveyCycleParam[] = "reset_survey_cycle";
 // static
 const char HatsFinchHelper::kResetAllParam[] = "reset_all";
+// static
+const char HatsFinchHelper::kTriggerIdParam[] = "trigger_id";
+
+std::string HatsFinchHelper::GetTriggerID() {
+  DCHECK(base::FeatureList::IsEnabled(features::kHappinessTrackingSystem));
+  return base::GetFieldTrialParamValueByFeature(
+      features::kHappinessTrackingSystem, kTriggerIdParam);
+}
 
 HatsFinchHelper::HatsFinchHelper(Profile* profile) : profile_(profile) {
   LoadFinchParamValues();
@@ -79,6 +87,8 @@ void HatsFinchHelper::LoadFinchParamValues() {
   first_survey_start_date_ =
       base::Time().FromJsTime(first_survey_start_date_ms);
 
+  trigger_id_ = GetTriggerID();
+
   reset_survey_cycle_ = base::GetFieldTrialParamByFeatureAsBool(
       feature, kResetSurveyCycleParam, false);
 
@@ -137,6 +147,11 @@ void HatsFinchHelper::CheckForDeviceSelection() {
   bool is_selected = false;
   if (rand_double < probability_of_pick_)
     is_selected = true;
+
+  // Check if the trigger id is a valid string. Trigger IDs are a hash strings
+  // of around 26 characters.
+  is_selected = is_selected && (trigger_id_.length() > 15);
+
   pref_service->SetBoolean(prefs::kHatsDeviceIsSelected, is_selected);
   device_is_selected_for_cycle_ = is_selected;
 }

chrome/browser/chromeos/hats/hats_finch_helper.h

@@ -19,6 +19,8 @@ namespace chromeos {
 // information related to the hats finch experiment.
 class HatsFinchHelper {
  public:
+  static std::string GetTriggerID();
+
   explicit HatsFinchHelper(Profile* profile);
   ~HatsFinchHelper();
 
@@ -39,6 +41,7 @@ class HatsFinchHelper {
   static const char kSurveyStartDateMsParam[];
   static const char kResetSurveyCycleParam[];
   static const char kResetAllParam[];
+  static const char kTriggerIdParam[];
 
   // Loads all the param values from the finch seed and initializes the member
   // variables.
@@ -70,6 +73,12 @@ class HatsFinchHelper {
   // key "survey_start_date_ms".
   base::Time first_survey_start_date_;
 
+  // The survey's trigger id. This is used by the Hats server to identify its
+  // client, Chrome OS in this case. Different Chrome OS surveys can have
+  // different trigger ids. This is provided as a param via the finch seed under
+  // the key "trigger_id".
+  std::string trigger_id_;
+
   // Indicates that the survey cycle needs to be reset if set to true.
   bool reset_survey_cycle_ = false;
 

chrome/browser/chromeos/hats/hats_finch_helper_unittest.cc

@@ -18,6 +18,9 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
+namespace {
+constexpr char kValidTriggerId[] = "1gksUIDXA0jBnuK8T6R0NfspWBvA";
+}  // namespace
 
 class HatsFinchHelperTest : public testing::Test {
  public:
@@ -32,13 +35,15 @@ class HatsFinchHelperTest : public testing::Test {
                                         std::string cycle_length,
                                         std::string start_date,
                                         std::string reset_survey,
-                                        std::string reset) {
+                                        std::string reset,
+                                        std::string trigger_id) {
     base::FieldTrialParams params;
     params[HatsFinchHelper::kProbabilityParam] = prob;
     params[HatsFinchHelper::kSurveyCycleLengthParam] = cycle_length;
     params[HatsFinchHelper::kSurveyStartDateMsParam] = start_date;
     params[HatsFinchHelper::kResetSurveyCycleParam] = reset_survey;
     params[HatsFinchHelper::kResetAllParam] = reset;
+    params[HatsFinchHelper::kTriggerIdParam] = trigger_id;
     return params;
   }
 
@@ -56,8 +61,8 @@ class HatsFinchHelperTest : public testing::Test {
 };
 
 TEST_F(HatsFinchHelperTest, InitFinchSeed_ValidValues) {
-  base::FieldTrialParams params =
-      CreateParamMap("1.0", "7", "1475613895337", "false", "false");
+  base::FieldTrialParams params = CreateParamMap(
+      "1.0", "7", "1475613895337", "false", "false", kValidTriggerId);
   SetFeatureParams(params);
 
   HatsFinchHelper hats_finch_helper(&profile_);
@@ -66,13 +71,14 @@ TEST_F(HatsFinchHelperTest, InitFinchSeed_ValidValues) {
   EXPECT_EQ(hats_finch_helper.survey_cycle_length_, 7);
   EXPECT_EQ(hats_finch_helper.first_survey_start_date_,
             base::Time().FromJsTime(1475613895337LL));
+  EXPECT_EQ(hats_finch_helper.trigger_id_, kValidTriggerId);
   EXPECT_FALSE(hats_finch_helper.reset_survey_cycle_);
   EXPECT_FALSE(hats_finch_helper.reset_hats_);
 }
 
 TEST_F(HatsFinchHelperTest, InitFinchSeed_Invalidalues) {
   base::FieldTrialParams params =
-      CreateParamMap("-0.1", "-1", "-1000", "false", "false");
+      CreateParamMap("-0.1", "-1", "-1000", "false", "false", "1A2B3C4D5");
   SetFeatureParams(params);
 
   base::Time current_time = base::Time::Now();
@@ -88,7 +94,7 @@ TEST_F(HatsFinchHelperTest, TestComputeNextDate) {
   base::FieldTrialParams params =
       CreateParamMap("0",
                      "7",  // 7 Days survey cycle length
-                     "0", "false", "false");
+                     "0", "false", "false", kValidTriggerId);
 
   SetFeatureParams(params);
 
@@ -116,7 +122,7 @@ TEST_F(HatsFinchHelperTest, TestComputeNextDate) {
 
 TEST_F(HatsFinchHelperTest, ResetSurveyCycle) {
   base::FieldTrialParams params =
-      CreateParamMap("0.5", "7", "1475613895337", "true", "0");
+      CreateParamMap("0.5", "7", "1475613895337", "true", "0", kValidTriggerId);
   SetFeatureParams(params);
 
   int64_t initial_timestamp = base::Time::Now().ToInternalValue();
@@ -140,7 +146,7 @@ TEST_F(HatsFinchHelperTest, ResetSurveyCycle) {
 
 TEST_F(HatsFinchHelperTest, ResetHats) {
   base::FieldTrialParams params =
-      CreateParamMap("0.5", "7", "1475613895337", "0", "true");
+      CreateParamMap("0.5", "7", "1475613895337", "0", "true", kValidTriggerId);
   SetFeatureParams(params);
 
   int64_t initial_timestamp = base::Time::Now().ToInternalValue();

chrome/browser/chromeos/hats/hats_notification_controller.cc

@@ -180,9 +180,7 @@ void HatsNotificationController::Click(
 
   UpdateLastInteractionTime();
 
-  // The dialog deletes itself on close.
-  HatsDialog::CreateAndShow(
-      gaia::IsGoogleInternalAccountEmail(profile_->GetProfileUserName()));
+  hats_dialog_ = HatsDialog::CreateAndShow();
 
   // Remove the notification.
   network_portal_detector::GetInstance()->RemoveObserver(this);

chrome/browser/chromeos/hats/hats_notification_controller.h

@@ -20,6 +20,7 @@ class Profile;
 class NetworkState;
 
 namespace chromeos {
+class HatsDialog;
 
 // Happiness tracking survey (HaTS) notification controller is responsible for
 // managing the HaTS notification that is displayed to the user.
@@ -67,6 +68,7 @@ class HatsNotificationController : public message_center::NotificationDelegate,
 
   Profile* const profile_;
   std::unique_ptr<message_center::Notification> notification_;
+  std::unique_ptr<HatsDialog> hats_dialog_;
   base::WeakPtrFactory<HatsNotificationController> weak_pointer_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(HatsNotificationController);

chrome/browser/resources/chromeos/hats/hats.html

-<html>
-  <head>
-    <style>
-      .done-button {
-        bottom: 17px;
-        color: rgb(33, 150, 243);
-        display: none;
-        filter: none;
-        font: 400 16px Roboto,RobotoDraft,Helvetica,sans-serif;
-        position: fixed;
-        right: 25px;
-        text-shadow: none;
-        text-transform: uppercase;
-        z-index: 10;
-      }
-    </style>
-    <script>
-      /**
-       * Handles the callback when the survey is submitted or when the survey
-       * has already been submitted in the past.
-       * @param {boolean} isFirstRun Will be true when the user just earned
-       *    access to the content and false if the user had already had access
-       *    previously.
-       */
-      var didFinishSurvey = function(isFirstRun) {
-        if (!isFirstRun) {
-          return;
-        }
-
-        /* Display the done button at the end of the survey. */
-        document.getElementById('id-done-button').style.display = 'block';
-      };
-
-      document.addEventListener('DOMContentLoaded', function() {
-        document.getElementById('id-link-done-button').onclick = function() {
-          window.close();
-        }
-      })
-    </script>
-    <script src="$SCRIPT_SRC"></script>
-  </head>
-  <body>
-    <a id="id-link-done-button" href="#" role="button">
-      <div id="id-done-button" class="done-button">$DONE_BUTTON_LABEL</div>
-    </a>
-  </body>
-</html>

chrome/browser/ui/views/hats/hats_next_web_dialog.cc

@@ -30,6 +30,7 @@
 #include "ui/views/bubble/bubble_frame_view.h"
 #include "ui/views/controls/webview/web_dialog_view.h"
 #include "ui/views/layout/fill_layout.h"
+#include "ui/web_dialogs/web_dialog_delegate.h"
 
 // A delegate used to intercept the creation of new WebContents by the HaTS
 // Next dialog.
@@ -166,6 +167,10 @@ bool HatsNextWebDialog::HandleContextMenu(
     const content::ContextMenuParams& params) {
   return true;
 }
+ui::WebDialogDelegate::FrameKind HatsNextWebDialog::GetWebDialogFrameKind()
+    const {
+  return ui::WebDialogDelegate::FrameKind::kDialog;
+}
 
 gfx::Size HatsNextWebDialog::CalculatePreferredSize() const {
   return size_;
@@ -198,8 +203,7 @@ HatsNextWebDialog::HatsNextWebDialog(Browser* browser,
 
   SetLayoutManager(std::make_unique<views::FillLayout>());
   web_view_ = AddChildView(std::make_unique<views::WebDialogView>(
-      otr_profile_, this, std::make_unique<ChromeWebContentsHandler>(),
-      /* use_dialog_frame */ true));
+      otr_profile_, this, std::make_unique<ChromeWebContentsHandler>()));
   set_margins(gfx::Insets());
   widget_ = views::BubbleDialogDelegateView::CreateBubble(this);
 

chrome/browser/ui/views/hats/hats_next_web_dialog.h

@@ -51,6 +51,7 @@ class HatsNextWebDialog : public ui::WebDialogDelegate,
   bool ShouldShowDialogTitle() const override;
   bool HandleContextMenu(content::RenderFrameHost* render_frame_host,
                          const content::ContextMenuParams& params) override;
+  ui::WebDialogDelegate::FrameKind GetWebDialogFrameKind() const override;
 
   // BubbleDialogDelegateView:
   gfx::Size CalculatePreferredSize() const override;

chrome/browser/ui/views/hats/hats_web_dialog.cc

@@ -28,6 +28,7 @@
 #include "ui/gfx/geometry/size.h"
 #include "ui/views/controls/webview/web_dialog_view.h"
 #include "ui/views/widget/widget.h"
+#include "ui/web_dialogs/web_dialog_delegate.h"
 #include "url/url_canon.h"
 #include "url/url_util.h"
 
@@ -196,6 +197,10 @@ void HatsWebDialog::OnMainFrameResourceLoadComplete(
   }
 }
 
+ui::WebDialogDelegate::FrameKind HatsWebDialog::GetWebDialogFrameKind() const {
+  return ui::WebDialogDelegate::FrameKind::kDialog;
+}
+
 views::View* HatsWebDialog::GetContentsView() {
   return webview_;
 }
@@ -231,8 +236,7 @@ void HatsWebDialog::CreateWebDialog(Browser* browser) {
   // Create a web dialog aligned to the bottom center of the location bar.
   SetButtons(ui::DIALOG_BUTTON_NONE);
   webview_ = new views::WebDialogView(
-      otr_profile_, this, std::make_unique<ChromeWebContentsHandler>(),
-      /* use_dialog_frame= */ true);
+      otr_profile_, this, std::make_unique<ChromeWebContentsHandler>());
   webview_->SetPreferredSize(
       gfx::Size(kDefaultHatsDialogWidth, kDefaultHatsDialogHeight));
   preloading_widget_ = constrained_window::CreateBrowserModalDialogViews(

chrome/browser/ui/views/hats/hats_web_dialog.h

@@ -63,6 +63,7 @@ class HatsWebDialog : public ui::WebDialogDelegate,
   void OnWebContentsFinishedLoad() override;
   void OnMainFrameResourceLoadComplete(
       const blink::mojom::ResourceLoadInfo& resource_load_info) override;
+  ui::WebDialogDelegate::FrameKind GetWebDialogFrameKind() const override;
 
   // views::DialogDelegateView implementation.
   views::View* GetContentsView() override;

ui/views/controls/webview/web_dialog_view.cc

@@ -74,13 +74,11 @@ void ObservableWebView::ResetDelegate() {
 
 WebDialogView::WebDialogView(content::BrowserContext* context,
                              WebDialogDelegate* delegate,
-                             std::unique_ptr<WebContentsHandler> handler,
-                             bool use_dialog_frame)
+                             std::unique_ptr<WebContentsHandler> handler)
     : ClientView(nullptr, nullptr),
       WebDialogWebContentsDelegate(context, std::move(handler)),
       delegate_(delegate),
-      web_view_(new ObservableWebView(context, delegate)),
-      use_dialog_frame_(use_dialog_frame) {
+      web_view_(new ObservableWebView(context, delegate)) {
   SetCanMinimize(!delegate_ || delegate_->can_minimize());
   SetModalType(GetDialogModalType());
   web_view_->set_allow_accelerators(true);
@@ -228,8 +226,18 @@ views::ClientView* WebDialogView::CreateClientView(views::Widget* widget) {
 
 std::unique_ptr<NonClientFrameView> WebDialogView::CreateNonClientFrameView(
     Widget* widget) {
-  return use_dialog_frame_ ? DialogDelegate::CreateDialogFrameView(widget)
-                           : WidgetDelegate::CreateNonClientFrameView(widget);
+  if (!delegate_)
+    return WidgetDelegate::CreateNonClientFrameView(widget);
+
+  switch (delegate_->GetWebDialogFrameKind()) {
+    case WebDialogDelegate::FrameKind::kNonClient:
+      return WidgetDelegate::CreateNonClientFrameView(widget);
+    case WebDialogDelegate::FrameKind::kDialog:
+      return DialogDelegate::CreateDialogFrameView(widget);
+    default:
+      NOTREACHED() << "Unknown frame kind type enum specified.";
+      return std::unique_ptr<NonClientFrameView>{};
+  }
 }
 
 views::View* WebDialogView::GetInitiallyFocusedView() {

ui/views/controls/webview/web_dialog_view.h

@@ -78,8 +78,7 @@ class WEBVIEW_EXPORT WebDialogView : public ClientView,
   // client frame view.
   WebDialogView(content::BrowserContext* context,
                 ui::WebDialogDelegate* delegate,
-                std::unique_ptr<WebContentsHandler> handler,
-                bool use_dialog_frame = false);
+                std::unique_ptr<WebContentsHandler> handler);
   ~WebDialogView() override;
 
   content::WebContents* web_contents();
@@ -206,9 +205,6 @@ class WEBVIEW_EXPORT WebDialogView : public ClientView,
   // Handler for unhandled key events from renderer.
   UnhandledKeyboardEventHandler unhandled_keyboard_event_handler_;
 
-  // Whether to use dialog frame view for non client frame view.
-  bool use_dialog_frame_ = false;
-
   bool disable_url_load_for_test_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(WebDialogView);

ui/web_dialogs/web_dialog_delegate.cc

@@ -85,4 +85,8 @@ bool WebDialogDelegate::CheckMediaAccessPermission(
   return false;
 }
 
+WebDialogDelegate::FrameKind WebDialogDelegate::GetWebDialogFrameKind() const {
+  return WebDialogDelegate::FrameKind::kNonClient;
+}
+
 }  // namespace ui

ui/web_dialogs/web_dialog_delegate.h

@@ -36,6 +36,11 @@ class Accelerator;
 // Implement this class to receive notifications.
 class WEB_DIALOGS_EXPORT WebDialogDelegate {
  public:
+  enum class FrameKind {
+    kDialog,     // Does not include a title bar or frame caption buttons.
+    kNonClient,  // Includes a non client frame view with title & buttons.
+  };
+
   // Returns the modal type for this dialog. Only called once, during
   // WebDialogView creation.
   virtual ModalType GetDialogModalType() const = 0;
@@ -184,6 +189,9 @@ class WEB_DIALOGS_EXPORT WebDialogDelegate {
       const GURL& security_origin,
       blink::mojom::MediaStreamType type);
 
+  // Whether to use dialog frame view for non client frame view.
+  virtual FrameKind GetWebDialogFrameKind() const;
+
   virtual ~WebDialogDelegate() = default;
 
  private: