Fall back to existing permission prompt when there's no chip string.

When the Permission Chip is enabled, you get a broken-looking UI when
there's no short string written. This patch works around this by falling
back to the old UI.

Bug: 1019129
Change-Id: If214734f780a0423c4ef9bdb0198c433cffc3cae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2431429
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811075}

chrome/browser/ui/views/location_bar/permission_chip.cc

@@ -259,7 +259,7 @@ base::string16 PermissionChip::GetPermissionMessage() {
   auto requests = delegate_->Requests();
 
   return requests.size() == 1
-             ? requests[0]->GetChipText()
+             ? requests[0]->GetChipText().value()
              : l10n_util::GetStringUTF16(
                    IDS_MEDIA_CAPTURE_VIDEO_AND_AUDIO_PERMISSION_CHIP);
 }

chrome/browser/ui/views/permission_bubble/permission_prompt_impl.cc

@@ -13,6 +13,7 @@
 #include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/permission_bubble/permission_prompt_bubble_view.h"
+#include "components/permissions/permission_request.h"
 #include "components/permissions/permission_request_manager.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/views/bubble/bubble_frame_view.h"
@@ -51,7 +52,12 @@ PermissionPromptImpl::PermissionPromptImpl(Browser* browser,
     content_settings::UpdateLocationBarUiForWebContents(web_contents_);
   } else {
     LocationBarView* lbv = GetLocationBarView();
-    if (base::FeatureList::IsEnabled(features::kPermissionChip) && lbv) {
+    std::vector<permissions::PermissionRequest*> requests =
+        delegate->Requests();
+    if (base::FeatureList::IsEnabled(features::kPermissionChip) && lbv &&
+        std::all_of(requests.begin(), requests.end(), [](auto* request) {
+          return request->GetChipText().has_value();
+        })) {
       permission_chip_ = lbv->permission_chip();
       permission_chip_->Show(delegate);
       prompt_style_ = PromptStyle::kChip;

components/permissions/permission_request.cc

@@ -19,8 +19,8 @@ ContentSettingsType PermissionRequest::GetContentSettingsType() const {
 }
 
 #if !defined(OS_ANDROID)
-base::string16 PermissionRequest::GetChipText() const {
-  return base::string16();
+base::Optional<base::string16> PermissionRequest::GetChipText() const {
+  return base::nullopt;
 }
 #endif
 

components/permissions/permission_request.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_PERMISSIONS_PERMISSION_REQUEST_H_
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/strings/string16.h"
 #include "build/build_config.h"
 #include "components/content_settings/core/common/content_settings_types.h"
@@ -115,7 +116,7 @@ class PermissionRequest {
 
 #if !defined(OS_ANDROID)
   // Returns the short text for the chip button related to this permission.
-  virtual base::string16 GetChipText() const;
+  virtual base::Optional<base::string16> GetChipText() const;
 #endif
 
   // Returns the shortened prompt text for this permission. The permission

components/permissions/permission_request_impl.cc

@@ -253,7 +253,7 @@ base::string16 PermissionRequestImpl::GetMessageTextFragment() const {
 }
 
 #if !defined(OS_ANDROID)
-base::string16 PermissionRequestImpl::GetChipText() const {
+base::Optional<base::string16> PermissionRequestImpl::GetChipText() const {
   int message_id;
   switch (content_settings_type_) {
     case ContentSettingsType::GEOLOCATION:
@@ -284,8 +284,10 @@ base::string16 PermissionRequestImpl::GetChipText() const {
       message_id = IDS_IDLE_DETECTION_PERMISSION_CHIP;
       break;
     default:
-      NOTREACHED();
-      return base::string16();
+      // TODO(bsep): We don't actually want to support having no string in the
+      // long term, but writing them takes time. In the meantime, we fall back
+      // to the existing UI when the string is missing.
+      return base::nullopt;
   }
   return l10n_util::GetStringUTF16(message_id);
 }

components/permissions/permission_request_impl.h

@@ -7,6 +7,7 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "build/build_config.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_types.h"
@@ -41,7 +42,7 @@ class PermissionRequestImpl : public PermissionRequest {
   base::string16 GetQuietMessageText() const override;
 #endif
 #if !defined(OS_ANDROID)
-  base::string16 GetChipText() const override;
+  base::Optional<base::string16> GetChipText() const override;
 #endif
   base::string16 GetMessageTextFragment() const override;
   GURL GetOrigin() const override;