Extensions: Change logic for ExtensionResource::empty().

ExtensionResource::empty() currently checks whether the extension root
path is empty. However this is not correct since dummy extension objects
are often created with an empty extension root path, for example, this
can happen during the webstore install flow. This can cause
ExtensionResource::empty() to incorrectly return false for a valid
ExtensionResource.

Fix this by changing ExtensionResource::empty() to check for the
emptiness of the relative path instead.

BUG=1087348

Change-Id: I195c6274118ba4bc869027dd6f53fde7199e3d4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225896Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#773922}

extensions/common/api/declarative_net_request/dnr_manifest_handler.cc

@@ -28,19 +28,6 @@ namespace dnr_api = api::declarative_net_request;
 
 namespace declarative_net_request {
 
-namespace {
-
-bool IsEmptyExtensionResource(const ExtensionResource& resource) {
-  // Note that just checking for ExtensionResource::empty() isn't correct since
-  // it checks |ExtensionResource::extension_root()::empty()| which can return
-  // true for a dummy extension created as part of the webstore installation
-  // flow. See crbug.com/1087348.
-  return resource.extension_id().empty() && resource.extension_root().empty() &&
-         resource.relative_path().empty();
-}
-
-}  // namespace
-
 DNRManifestHandler::DNRManifestHandler() = default;
 DNRManifestHandler::~DNRManifestHandler() = default;
 
@@ -96,8 +83,7 @@ bool DNRManifestHandler::Parse(Extension* extension, base::string16* error) {
                               int index, DNRManifestData::RulesetInfo* info) {
     // Path validation.
     ExtensionResource resource = extension->GetResource(rulesets[index].path);
-    if (IsEmptyExtensionResource(resource) ||
-        resource.relative_path().ReferencesParent()) {
+    if (resource.empty() || resource.relative_path().ReferencesParent()) {
       *error = ErrorUtils::FormatErrorMessageUTF16(
           errors::kRulesFileIsInvalid, keys::kDeclarativeNetRequestKey,
           keys::kDeclarativeRuleResourcesKey, rulesets[index].path);

extensions/common/extension_resource.h

@@ -11,10 +11,11 @@
 
 namespace extensions {
 
-// Represents a resource inside an extension. For example, an image, or a
-// JavaScript file. This is more complicated than just a simple FilePath
-// because extension resources can come from multiple physical file locations
-// depending on locale.
+// Represents a resource inside an extension. Hence a resource pointing to the
+// root extension directory isn't a valid ExtensionResource.
+// Examples include an image, or a JavaScript file. This is more complicated
+// than just a simple FilePath because extension resources can come from
+// multiple physical file locations depending on locale.
 class ExtensionResource {
  public:
   // SymlinkPolicy decides whether we'll allow resources to be a symlink to
@@ -58,10 +59,14 @@ class ExtensionResource {
 
   // Getters
   const std::string& extension_id() const { return extension_id_; }
+
+  // Note that this might be empty for a valid ExtensionResource since dummy
+  // Extension objects may be created with an empty extension root path in code.
   const base::FilePath& extension_root() const { return extension_root_; }
+
   const base::FilePath& relative_path() const { return relative_path_; }
 
-  bool empty() const { return extension_root().empty(); }
+  bool empty() const { return relative_path().empty(); }
 
   // Unit test helpers.
   base::FilePath::StringType NormalizeSeperators(