diff --git a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
index 98ead5e..457089b 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
@@ -25,10 +25,9 @@
 import java.util.*;
 
 import org.apache.felix.framework.cache.BundleArchive;
+import org.apache.felix.framework.ext.SecurityProvider;
 import org.apache.felix.framework.searchpolicy.ModuleImpl;
 import org.apache.felix.framework.searchpolicy.URLPolicyImpl;
-import org.apache.felix.framework.util.manifestparser.ManifestParser;
-import org.apache.felix.framework.util.manifestparser.R4Library;
 import org.apache.felix.moduleloader.IModule;
 import org.osgi.framework.*;
 
@@ -69,14 +68,18 @@
         m_activator = null;
         m_context = null;
 
-        // TODO: REFACTOR - Null check is a hack due to system bundle.
+        // We have to check for null here because the framework/system bundle
+        // extends BundleImpl and it doesn't have an archive.
         if (m_archive != null)
         {
             addModule(createModule());
         }
     }
 
-    // TODO: REFACTOR - We need this method so the system bundle can override it.
+    // This method exists because the system bundle extends BundleImpl
+    // and cannot pass itself into the BundleImpl constructor. All methods
+    // in BundleImpl should use this method to get the framework and should
+    // not access the field directly.
     Felix getFramework()
     {
         return m_felix;
@@ -883,9 +886,19 @@
         return m_archive.rollbackRevise();
     }
 
-    // TODO: REFACTOR - This module is only visible for the system bundle.
-    synchronized void addModule(IModule module)
+    // This method should be private, but is visible because the
+    // system bundle needs to add its module directly to the bundle,
+    // since it doesn't have an archive from which the module will
+    // be created, which is the normal case.
+    synchronized void addModule(IModule module) throws Exception
     {
+        SecurityProvider sp = getFramework().getSecurityProvider();
+        if (sp != null)
+        {
+            sp.checkBundle(this);
+        }
+        module.setSecurityContext(new BundleProtectionDomain(m_felix, this));
+
         ((ModuleImpl) module).setBundle(this);
 
         IModule[] dest = new IModule[m_modules.length + 1];
diff --git a/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java b/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
index 4cf3367..9fc10a0 100644
--- a/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
+++ b/framework/src/main/java/org/apache/felix/framework/ExtensionManager.java
@@ -697,16 +697,6 @@
             return m_urlPolicy;
         }
 
-        public void setSecurityContext(Object securityContext)
-        {
-            throw new UnsupportedOperationException("Should not be used!");
-        }
-
-        public Object getSecurityContext()
-        {
-            throw new UnsupportedOperationException("Should not be used!");
-        }
-
         public Class findClassByDelegation(String name) throws ClassNotFoundException
         {
             return getClassFromModule(name);
diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java b/framework/src/main/java/org/apache/felix/framework/Felix.java
index 65561d9..9b819a1 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -298,15 +298,6 @@
         m_extensionManager = new ExtensionManager(m_logger, m_configMap);
         addModule(m_extensionManager.getModule());
         m_resolverState.addModule(getCurrentModule());
-        // Lastly, set the system bundle's protection domain.
-        try
-        {
-            addSecurity(this);
-        }
-        catch (Exception ex)
-        {
-            // This should not happen
-        }
     }
 
     Logger getLogger()
@@ -349,9 +340,8 @@
         return result;
     }
 
-    // TODO: REFACTOR - This method is sort of a hack. Since the system bundle
-    //       can't set its own framework value, it can override this method to
-    //       return itself to the BundleImpl methods.
+    // This overrides the default behavior of BundleImpl.getFramework()
+    // to return "this", since the system bundle is the framework.
     Felix getFramework()
     {
         return this;
@@ -1603,7 +1593,6 @@
                         Util.isExtensionBundle(
                             bundle.getCurrentModule().getHeaders()))
                     {
-                        addSecurity(bundle);
                         m_extensionManager.addExtensionBundle(this, bundle);
 // TODO: REFACTOR - REFRESH RESOLVER STATE.
 //                        m_factory.refreshModule(m_sbi.getCurrentModule());
@@ -1613,10 +1602,6 @@
                     {
                         bundle.setState(Bundle.INSTALLED);
                     }
-                    else
-                    {
-                        addSecurity(bundle);
-                    }
                 }
                 catch (Throwable ex)
                 {
@@ -2051,8 +2036,6 @@
 
                 verifyExecutionEnvironment(bundle);
 
-                addSecurity(bundle);
-
                 if (!bundle.isExtension())
                 {
                     Object sm = System.getSecurityManager();
@@ -3049,6 +3032,11 @@
 
     private volatile SecurityProvider m_securityProvider;
 
+    SecurityProvider getSecurityProvider()
+    {
+        return m_securityProvider;
+    }
+
     void setSecurityProvider(SecurityProvider securityProvider)
     {
         m_securityProvider = securityProvider;
@@ -3072,17 +3060,6 @@
         return true;
     }
 
-    // TODO: SECURITY - This should probably take a module, not a bundle.
-    void addSecurity(final BundleImpl bundle) throws Exception
-    {
-        if (m_securityProvider != null)
-        {
-            m_securityProvider.checkBundle(bundle);
-        }
-        bundle.getCurrentModule().setSecurityContext(
-            new BundleProtectionDomain(this, bundle));
-    }
-
     private BundleActivator createBundleActivator(BundleImpl impl)
         throws Exception
     {
@@ -3797,8 +3774,6 @@
                     // Add new module to resolver state.
 // TODO: REFACTOR - It is not clean to have to remember these steps.
                     m_resolverState.addModule(oldImpl.getCurrentModule());
-// TODO: REFACTOR - Could we set this when we add the module to the bundle?.
-                    addSecurity(m_bundle);
                     fireBundleEvent(BundleEvent.UNRESOLVED, m_bundle);
                 }
                 catch (Exception ex)
diff --git a/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java b/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java
index f3fd48b..42ce3ee 100644
--- a/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/searchpolicy/ModuleImpl.java
@@ -736,7 +736,6 @@
         }
     }
 
-// TODO: REFACTOR - This previously didn't cause the class loader to be created.
     public URL getResourceFromModule(String name)
     {
         URL url = null;
@@ -769,7 +768,6 @@
         return url;
     }
 
-// TODO: REFACTOR - This previously didn't cause the class loader to be created.
     public Enumeration getResourcesFromModule(String name)
     {
         Vector v = new Vector();