Extract supported codecs from CDM component manifest.

chrome/browser/component_updater/default_component_installer.cc

 // Copyright 2013 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 "base/bind.h"
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/values.h"
 #include "base/version.h"
 #include "chrome/browser/component_updater/default_component_installer.h"
 #include "content/public/browser/browser_thread.h"
 
 namespace {
 // Version "0" corresponds to no installed version. By the server's conventions,
 // we represent it as a dotted quad.
 const char kNullVersion[] = "0.0.0.0";
 }  // namespace
 
+ComponentInstallerTraits::~ComponentInstallerTraits() {
+}
+
 DefaultComponentInstaller::DefaultComponentInstaller(
     scoped_ptr<ComponentInstallerTraits> installer_traits)
     : current_version_(kNullVersion) {
   installer_traits_ = installer_traits.Pass();
 }
 
 DefaultComponentInstaller::~DefaultComponentInstaller() {
 }
 
 void DefaultComponentInstaller::Register(ComponentUpdateService* cus) {
@@ skipping 39 matching lines @@
       installer_traits_->GetBaseDirectory().AppendASCII(version.GetString());
   if (base::PathExists(install_path)) {
     if (!base::DeleteFile(install_path, true))
       return false;
   }
   if (!InstallHelper(manifest, unpack_path, install_path)) {
     base::DeleteFile(install_path, true);
     return false;
   }
   current_version_ = version;
-  installer_traits_->ComponentReady(current_version_, GetInstallDirectory());
+  current_manifest_.reset(manifest.DeepCopy());
+  installer_traits_->ComponentReady(
+      current_version_,
+      GetInstallDirectory(),
+      scoped_ptr<base::DictionaryValue>(current_manifest_->DeepCopy()).Pass());
   return true;
 }
 
 bool DefaultComponentInstaller::GetInstalledFile(
     const std::string& file,
     base::FilePath* installed_file) {
   if (current_version_.Equals(base::Version(kNullVersion)))
     return false;  // No component has been installed yet.
 
   *installed_file =
@@ skipping 36 matching lines @@
       }
     } else {
       latest_dir = path;
       latest_version = version;
       found = true;
     }
   }
 
   if (found) {
     current_version_ = latest_version;
+    // TODO(waffles): Why do we save these as members rather than passing them
+    // to FinishRegistration()?

[waffles, 2013/10/24 23:07:06]

There was the thought that the installer would be the authority on what version
was installed - fingerprint is a stronger identifier than version, so it was
assumed that the installer would need to know about that as well, but in
practice all it does it hand it off to CUS for update checks, as you observe.

It is fine to pass current_fingerprint_ and current_manifest_ to
FinishRegistration rather than save it.

[ddorwin, 2013/10/25 00:06:05]

Okay, I'll change the TODO to be for me to remove them.

     base::ReadFileToString(latest_dir.AppendASCII("manifest.fingerprint"),
                            &current_fingerprint_);
+    current_manifest_.reset(ReadComponentManifest(latest_dir));
+    if (!current_manifest_) {
+      DLOG(ERROR) << "Failed to read manifest for "
+                  << installer_traits_->GetName() << " ("
+                  << base_dir.MaybeAsASCII() << ").";
+      return;
+    }
   }
 
   // Remove older versions of the component. None should be in use during
   // browser startup.
   for (std::vector<base::FilePath>::iterator iter = older_dirs.begin();
        iter != older_dirs.end(); ++iter) {
    base::DeleteFile(*iter, true);
   }
 
   content::BrowserThread::PostTask(
@@ skipping 19 matching lines @@
   installer_traits_->GetHash(&crx.pk_hash);
   ComponentUpdateService::Status status = cus->RegisterComponent(crx);
   if (status != ComponentUpdateService::kOk &&
       status != ComponentUpdateService::kReplaced) {
     NOTREACHED() << "Component registration failed for "
                  << installer_traits_->GetName();
     return;
   }
 
   if (current_version_.CompareTo(base::Version(kNullVersion)) > 0) {
+    // TODO(waffles): Why is this function called on the FILE thread? The only
+    // implementation just posts back to the UI thread. Should we instead post
+    // to UI in Install()?

[waffles, 2013/10/24 23:07:06]

The thinking was that people would likely want to manipulate those files, e.g.
read the manifest.

What's the motivation for reading the manifest in DefaultComponentInstaller
instead of in Widevine's ComponentReady?

[ddorwin, 2013/10/25 00:06:05]

Assuming we're going to pass in the manifest like we do now, does it make sense
to call ComponentReady() on the UI thread, which it is already on in most cases,
and let individual Traits post if necessary?
Responsibility separation and consistency. The Traits interface and the WV
subclass don't know about the manifest location, name, format, etc., and it
doesn't make sense to duplicate that knowledge in each subclass. In the install
case, the DictionaryValue is passed to the Traits (because the manifest has
already been parsed). We could parse it in the WV class, but that would mean
unnecessarily re-parsing it in the install case. That is clearly much less
frequent of a case, but it still feels like the wrong design.

The downside is that we may unnecessarily read and parse the manifest for other
components. I originally made the parsing optional based on a Traits property,
but then I found we were already reading the fingerprint from disk for every
component. I decided to KISS and worry about optimization later if necessary.
(Currently there are no other subclasses.)

[waffles, 2013/10/25 16:53:42]

I'm OK with passing the manifest; it's fine with me to post to UI, too.

It's true that Widevine is the only user of this code, but PNaCl will be soon
(next week?), now that it is unblocked on on-demand.

On 2013/10/25 00:06:05, ddorwin wrote:

[ddorwin, 2013/10/25 19:03:40]

Updated TODO.
Do you have an indication of what they intend to do in ComponentReady? Maybe
I'll wait and see before addressing the TODO.

[waffles, 2013/10/25 19:43:45]

They'll call PathService::OverridePath and then some stuff on UI; so I think
it's safe to switch this to UI before then.

+    scoped_ptr<base::DictionaryValue> manifest_copy(
+        current_manifest_->DeepCopy());
     content::BrowserThread::PostTask(
         content::BrowserThread::FILE, FROM_HERE,
         base::Bind(&ComponentInstallerTraits::ComponentReady,
                    base::Unretained(installer_traits_.get()),
                    current_version_,
-                   GetInstallDirectory()));
+                   GetInstallDirectory(),
+                   base::Passed(&manifest_copy)));
   }
 }