New issue
Advanced search Search tips

Issue 666153 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

DictionaryValue leak in component_loader.cc

Project Member Reported by lazyboy@chromium.org, Nov 17 2016

Issue description

1. Clearly ComponentLoader::GetExtensionID() leaks |manifest| as ComponentExtensionInfo doesn't directly take ownership of the DictionaryValue.
However, the function is unused :)

2. ComponentLoader::Add(const base::DictionaryValue*, const base::FilePath&, bool) leaks DictionaryValue when it bails out early:
  ComponentExtensionInfo info(parsed_manifest, root_directory);
  if (!ignore_whitelist_for_testing_ &&
      !skip_whitelist &&
      !IsComponentExtensionWhitelisted(info.extension_id))
    return std::string(); // |parsed_manifest| is leaked. 
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 18 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4e5cdb1ac9b339725dae67d5f6af9df8c5fe6bb

commit f4e5cdb1ac9b339725dae67d5f6af9df8c5fe6bb
Author: lazyboy <lazyboy@chromium.org>
Date: Fri Nov 18 02:58:45 2016

Fix DictionaryValue leak in component_loader.cc

ComponentLoader::GetExtensionID() used to leak |manifest|, but this
function was unused, so removed it.

Component::Add(DictionaryValue*,FilePath&,bool) also leaks DictionaryValue
when it bails out early.

BUG= 666153 

Review-Url: https://codereview.chromium.org/2504333003
Cr-Commit-Position: refs/heads/master@{#433036}

[modify] https://crrev.com/f4e5cdb1ac9b339725dae67d5f6af9df8c5fe6bb/chrome/browser/extensions/component_loader.cc
[modify] https://crrev.com/f4e5cdb1ac9b339725dae67d5f6af9df8c5fe6bb/chrome/browser/extensions/component_loader.h
[modify] https://crrev.com/f4e5cdb1ac9b339725dae67d5f6af9df8c5fe6bb/chrome/browser/extensions/component_loader_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment