Move creation and ownership of DownloadManager from the embedder to content. This matches all the o…

content/public/browser/browser_context.h

 // Copyright (c) 2012 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.
 
 #ifndef CONTENT_PUBLIC_BROWSER_BROWSER_CONTEXT_H_
 #define CONTENT_PUBLIC_BROWSER_BROWSER_CONTEXT_H_
 #pragma once
 
 #include "base/hash_tables.h"
 #include "base/supports_user_data.h"
@@ skipping 19 matching lines @@
 namespace webkit_database {
 class DatabaseTracker;
 }
 
 class FilePath;
 
 namespace content {
 
 class DOMStorageContext;
 class DownloadManager;
+class DownloadManagerDelegate;
 class GeolocationPermissionContext;
 class IndexedDBContext;
 class ResourceContext;
 class SpeechRecognitionPreferences;
 
 // This class holds the context needed for a browsing session.
 // It lives on the UI thread.
 class CONTENT_EXPORT BrowserContext : public base::SupportsUserData {
  public:
+  static DownloadManager* GetDownloadManager(BrowserContext* browser_context);
   static quota::QuotaManager* GetQuotaManager(BrowserContext* browser_context);
   static DOMStorageContext* GetDOMStorageContext(
       BrowserContext* browser_context);
   static IndexedDBContext* GetIndexedDBContext(BrowserContext* browser_context);
   static webkit_database::DatabaseTracker* GetDatabaseTracker(
       BrowserContext* browser_context);
   static appcache::AppCacheService* GetAppCacheService(
       BrowserContext* browser_context);
   static fileapi::FileSystemContext* GetFileSystemContext(
       BrowserContext* browser_context);
@@ skipping 14 matching lines @@
 
   virtual ~BrowserContext();
 
   // Returns the path of the directory where this context's data is stored.
   virtual FilePath GetPath() = 0;
 
   // Return whether this context is incognito. Default is false.
   // This doesn't belong here; http://crbug.com/89628
   virtual bool IsOffTheRecord() const = 0;
 
-  // Returns the DownloadManager associated with this context.
-  virtual content::DownloadManager* GetDownloadManager() = 0;
-
   // Returns the request context information associated with this context.  Call
   // this only on the UI thread, since it can send notifications that should
   // happen on the UI thread.
   virtual net::URLRequestContextGetter* GetRequestContext() = 0;
 
   // Returns the request context appropriate for the given renderer. If the
   // renderer process doesn't have an associated installed app, or if the
   // installed app's is_storage_isolated() returns false, this is equivalent to
   // calling GetRequestContext().
   // TODO(creis): After isolated app storage is no longer an experimental
   // feature, consider making this the default contract for GetRequestContext.
   virtual net::URLRequestContextGetter* GetRequestContextForRenderProcess(
       int renderer_child_id) = 0;
 
   // Returns the request context for media resources associated with this
   // context.
   virtual net::URLRequestContextGetter* GetRequestContextForMedia() = 0;
 
   // Returns the resource context.
   virtual ResourceContext* GetResourceContext() = 0;
 
+  // Returns the DownloadManagerDelegate for this context. This will be called
+  // once per context. It's valid to return NULL.
+  virtual DownloadManagerDelegate* GetDownloadManagerDelegate() = 0;

[Randy Smith (Not in Mondays), 2012/06/06 15:51:59]

What's the conceptual reason for not having the DownloadManager accessible from
the BrowserContext?  My naive reading of the code makes it look like the
BrowserContext owns the DM now, and the DM is a content level concept, unlike
the DMD, which is an content/embedder interface concept.

(Asking not because I'm wanting this to change, but because I don't think I'll
be able to do a good review without understanding it.)

[jam, 2012/06/06 16:06:15]

The reason is that BrowserContext is an interface that the embedder implements.
When we started the chrome/content split, chrome still had to provide DM and a
lot of other classes (see lines 51-60 above). This places a lot of burden on
embedders to create these objects in the right order etc. The goal has been to
make embedding content easier, so we've moved ownership/creation of any object
that content needs to content.

So now since content creates this object, we don't want to ask the embedder for
something that content created. That, and also to follow the content API's
convention of having the embedder interfaces not have any implementations.
correct
right, for now DMD is owned by the embedder. we can change this if you like.

[Randy Smith (Not in Mondays), 2012/06/06 16:34:45]

Makes sense; thanks for the explanation.
Nope, that seems fine to me as long as we document it properly.

+
   // Returns the geolocation permission context for this context.
   virtual GeolocationPermissionContext* GetGeolocationPermissionContext() = 0;
 
   // Returns the speech input preferences. SpeechRecognitionPreferences is a
   // ref counted class, so callers should take a reference if needed.
   virtual SpeechRecognitionPreferences* GetSpeechRecognitionPreferences() = 0;
 
   // Returns true if the last time this context was open it was exited cleanly.
   // This doesn't belong here; http://crbug.com/90737
   virtual bool DidLastSessionExitCleanly() = 0;
@@ skipping 11 matching lines @@
 struct hash<content::BrowserContext*> {
   std::size_t operator()(content::BrowserContext* const& p) const {
     return reinterpret_cast<std::size_t>(p);
   }
 };
 
 }  // namespace BASE_HASH_NAMESPACE
 #endif
 
 #endif  // CONTENT_PUBLIC_BROWSER_BROWSER_CONTEXT_H_