Revert of Move the event attach/detach logic on unload from event.js to (patchset #7 id:130001 of https://codereview.chromium.org/1074273002/)

Reason for revert:
This is breaking ExtensionApiTest.EventsAreUnregistered on Win7 Dedubg bots.

See:
https://build.chromium.org/p/chromium.webkit/builders/Win7%20%28dbg%29/builds/16162

Original issue's description:
> Move the event attach/detach logic on unload from event.js to
> event_bindings.cc.
>
> This removes one of the reasons to call into JavaScript on context unload,
> which can crash. It's also more robust; it's confusing trying to maintain a
> data structure in JavaScript which reflects C++ state, which contributes to
> bugs like crbug.com/474718.
>
> Also clean up and formalise the script context invalidation system: CHECK
> rather than guarding against multiple invalidations, only invalidate script
> context related member variables, add an invalidation observer interface.
>
> BUG=474718, 475536
> R=rdevlin.cronin@chromium.org
>
> Committed: https://crrev.com/d99095034d2e88897ae82c8353f3327a3a1d03a5
> Cr-Commit-Position: refs/heads/master@{#324933}

TBR=rdevlin.cronin@chromium.org,kalman@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=474718, 475536

Review URL: https://codereview.chromium.org/1083663004

Cr-Commit-Position: refs/heads/master@{#325050}

1 files changed, 10 insertions, 18 deletions

diff --git a/extensions/renderer/module_system.cc b/extensions/renderer/module_system.cc
index 6a12074..6194b8e 100644
--- a/extensions/renderer/module_system.cc
+++ b/extensions/renderer/module_system.cc
@@ -6,7 +6,6 @@
 
 #include "base/bind.h"
 #include "base/command_line.h"
-#include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -152,10 +151,12 @@ ModuleSystem::ModuleSystem(ScriptContext* context, SourceMap* source_map)
   }
 }
 
-ModuleSystem::~ModuleSystem() {
-}
+ModuleSystem::~ModuleSystem() { Invalidate(); }
 
 void ModuleSystem::Invalidate() {
+  if (!is_valid())
+    return;
+
   // Clear the module system properties from the global context. It's polite,
   // and we use this as a signal in lazy handlers that we no longer exist.
   {
@@ -167,11 +168,12 @@ void ModuleSystem::Invalidate() {
         v8::String::NewFromUtf8(GetIsolate(), kModuleSystem));
   }
 
-  // Invalidate all active and clobbered NativeHandlers we own.
-  for (const auto& handler : native_handler_map_)
-    handler.second->Invalidate();
-  for (const auto& clobbered_handler : clobbered_native_handlers_)
-    clobbered_handler->Invalidate();
+  // Invalidate all of the successfully required handlers we own.
+  for (NativeHandlerMap::iterator it = native_handler_map_.begin();
+       it != native_handler_map_.end();
+       ++it) {
+    it->second->Invalidate();
+  }
 
   ObjectBackedNativeHandler::Invalidate();
 }
@@ -299,13 +301,11 @@ v8::Local<v8::Value> ModuleSystem::CallModuleMethod(
 void ModuleSystem::RegisterNativeHandler(
     const std::string& name,
     scoped_ptr<NativeHandler> native_handler) {
-  ClobberExistingNativeHandler(name);
   native_handler_map_[name] =
       linked_ptr<NativeHandler>(native_handler.release());
 }
 
 void ModuleSystem::OverrideNativeHandlerForTest(const std::string& name) {
-  ClobberExistingNativeHandler(name);
   overridden_native_handlers_.insert(name);
 }
 
@@ -685,12 +685,4 @@ void ModuleSystem::OnModuleLoaded(
   resolver_local->Resolve(value);
 }
 
-void ModuleSystem::ClobberExistingNativeHandler(const std::string& name) {
-  NativeHandlerMap::iterator existing_handler = native_handler_map_.find(name);
-  if (existing_handler != native_handler_map_.end()) {
-    clobbered_native_handlers_.push_back(existing_handler->second);
-    native_handler_map_.erase(existing_handler);
-  }
-}
-
 }  // namespace extensions