Make PermissionRequest::GetIconId return different types

chrome/browser/custom_handlers/register_protocol_handler_permission_request.cc

 // Copyright 2014 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 "chrome/browser/custom_handlers/register_protocol_handler_permission_request.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 #include "chrome/grit/generated_resources.h"
 #include "content/public/browser/user_metrics.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/vector_icons_public.h"
 
-#if defined(OS_MACOSX)
-#include "chrome/grit/theme_resources.h"
-#endif
-
 namespace {
 
 base::string16 GetProtocolName(
     const ProtocolHandler& handler) {
   if (handler.protocol() == "mailto")
     return l10n_util::GetStringUTF16(IDS_REGISTER_PROTOCOL_HANDLER_MAILTO_NAME);
   if (handler.protocol() == "webcal")
     return l10n_util::GetStringUTF16(IDS_REGISTER_PROTOCOL_HANDLER_WEBCAL_NAME);
   return base::UTF8ToUTF16(handler.protocol());
 }
 
 }  // namespace
 
 RegisterProtocolHandlerPermissionRequest
 ::RegisterProtocolHandlerPermissionRequest(
       ProtocolHandlerRegistry* registry,
       const ProtocolHandler& handler,
       GURL url,
       bool user_gesture)
     : registry_(registry),
       handler_(handler),
       origin_(url.GetOrigin()) {}
 
 RegisterProtocolHandlerPermissionRequest::
 ~RegisterProtocolHandlerPermissionRequest() {}
 
-gfx::VectorIconId RegisterProtocolHandlerPermissionRequest::GetVectorIconId()
+PermissionRequest::IconId RegisterProtocolHandlerPermissionRequest::GetIconId()
     const {
-#if defined(OS_MACOSX)
-  return gfx::VectorIconId::VECTOR_ICON_NONE;

[lshang, 2016/10/11 12:26:57]

Did you remove this because this file will never get compiled on MACOSX? I can
only see it's non-android from BUILD.gn file, but don't know if there's other
guarantee of it. Otherwise, it'll return  gfx::VectorIconId::PROTOCOL_HANDLER on
MACOSX, instead of gfx::VectorIconId::VECTOR_ICON_NONE after your change. Or is
this intended for some reason? 

[Evan Stade, 2016/10/12 15:12:36]

vector icons work fine on osx. At the time these ifdefs code were written, that
wasn't the case.

[lshang, 2016/10/13 05:03:10]

Are the visual representation of gfx::VectorIconId::PROTOCOL_HANDLER and
IDR_REGISTER_PROTOCOL_HANDLER the same when showing to users? I'm still a little
doubt about whether this will make a UI change, but since you are more informed
about this, I'll trust you in this. 

[Evan Stade, 2016/10/14 14:58:29]

any difference is an intentional update to better match the material design
style of iconography (I can't actually recall offhand if there is a difference
or not)

-#else
   return gfx::VectorIconId::PROTOCOL_HANDLER;
-#endif
-}
-
-int RegisterProtocolHandlerPermissionRequest::GetIconId() const {
-#if defined(OS_MACOSX)
-  return IDR_REGISTER_PROTOCOL_HANDLER;
-#else
-  return 0;
-#endif
 }
 
 base::string16
 RegisterProtocolHandlerPermissionRequest::GetMessageTextFragment() const {
   ProtocolHandler old_handler = registry_->GetHandlerFor(handler_.protocol());
   return old_handler.IsEmpty() ?
       l10n_util::GetStringFUTF16(
           IDS_REGISTER_PROTOCOL_HANDLER_CONFIRM_FRAGMENT,
           GetProtocolName(handler_)) :
       l10n_util::GetStringFUTF16(
@@ skipping 25 matching lines @@
 }
 
 void RegisterProtocolHandlerPermissionRequest::RequestFinished() {
   delete this;
 }
 
 PermissionRequestType
 RegisterProtocolHandlerPermissionRequest::GetPermissionRequestType() const {
   return PermissionRequestType::REGISTER_PROTOCOL_HANDLER;
 }