refactor to Introduce AUDIO_CAPTURE and VIDEO_CAPTURE permissions.

chrome/browser/permissions/permission_manager.cc

Index: chrome/browser/permissions/permission_manager.cc
diff --git a/chrome/browser/permissions/permission_manager.cc b/chrome/browser/permissions/permission_manager.cc
index e39955ab7c364015d180c9bb05e67b771d752814..8cd21445b6b5e610335881d61e94c95aa3d95dd1 100644
--- a/chrome/browser/permissions/permission_manager.cc
+++ b/chrome/browser/permissions/permission_manager.cc
@@ -63,6 +63,10 @@ ContentSettingsType PermissionTypeToContentSetting(PermissionType permission) {
     case PermissionType::MIDI:
       // This will hit the NOTREACHED below.
       break;
+    case PermissionType::AUDIO_CAPTURE:
+      return CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC;
+    case PermissionType::VIDEO_CAPTURE:
+      return CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA;
     case PermissionType::NUM:
       // This will hit the NOTREACHED below.

[palmer, 2015/09/01 18:31:04]

Same here: just put the default case inside the switch with an explicit default:
label.

(a) it's more straightforward ("The purpose of this function is to map 1 enum to
another, and nothing else"); (b) I wonder if the compiler finds it easier to
optimize such pure-switch functions.

[guoweis_left_chromium, 2015/09/01 19:36:34]

intentionally leave out default to have compiler catch the case when enum is
missing.

       break;
@@ -207,9 +211,8 @@ PermissionStatus PermissionManager::GetPermissionStatus(
   if (!context)
     return content::PERMISSION_STATUS_DENIED;
 
-  return ContentSettingToPermissionStatus(
-      context->GetPermissionStatus(requesting_origin.GetOrigin(),
-                                   embedding_origin.GetOrigin()));
+  return ContentSettingToPermissionStatus(context->GetPermissionStatus(
+      requesting_origin.GetOrigin(), embedding_origin.GetOrigin()));

[palmer, 2015/09/01 18:31:04]

I'm still really anxious about this. I believe it may be incorrect, given the
documented contract of GetOrigin:

166   // A helper function to return a GURL containing just the scheme, host,
167   // and port from a URL. Equivalent to clearing any username and password,
168   // replacing the path with a slash, and clearing everything after that. If
169   // this URL is not a standard URL, then the result will be an empty,
170   // invalid GURL. If the URL has neither username nor password, this
171   // degenerates to GetWithEmptyPath().
172   //
173   // It is an error to get the origin of an invalid URL. The result
174   // will be the empty URL.
175   GURL GetOrigin() const;

That is, all non-standard URLs will be equivalent, which is a violation of the
same origin policy. Note the implementation of the comparators:

182 bool GURL::operator==(const GURL& other) const {
183   return spec_ == other.spec_;
184 }
185 
186 bool GURL::operator!=(const GURL& other) const {
187   return spec_ != other.spec_;
188 }
189 
190 bool GURL::operator<(const GURL& other) const {
191   return spec_ < other.spec_;
192 }
193 
194 bool GURL::operator>(const GURL& other) const {
195   return spec_ > other.spec_;
196 }

[guoweis_left_chromium, 2015/09/01 19:36:34]

crbug.com/527149 filed for this.

 }
 
 void PermissionManager::RegisterPermissionUsage(PermissionType permission,