Make sure we don't crash on NULL pointer reference on late port open notification.

Also improve teardown, which should make sure this doesn't happen in the first place, may will close extension ports on frame teardown.

BUG=3171179 
BUG=3222287
TEST=unittests in change.


Review URL: http://codereview.chromium.org/5339008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67463 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 21 insertions, 3 deletions

diff --git a/ceee/ie/plugin/bho/extension_port_manager.cc b/ceee/ie/plugin/bho/extension_port_manager.cc
index 9bc28f5..2463023 100644
--- a/ceee/ie/plugin/bho/extension_port_manager.cc
+++ b/ceee/ie/plugin/bho/extension_port_manager.cc
@@ -140,7 +140,6 @@ void ExtensionPortManager::OnPortMessage(BSTR message) {
       // TODO(siggi@chromium.org): This can happen legitimately on a
       //    race between connect and document unload. We should
       //    probably respond with a close port message here.
-      NOTREACHED();
       LOG(ERROR) << "No such connection id " << connection_id;
       return;
     }
diff --git a/ceee/ie/plugin/scripting/content_script_native_api.cc b/ceee/ie/plugin/scripting/content_script_native_api.cc
index a74d077..79e8150 100644
--- a/ceee/ie/plugin/scripting/content_script_native_api.cc
+++ b/ceee/ie/plugin/scripting/content_script_native_api.cc
@@ -47,6 +47,10 @@ HRESULT ContentScriptNativeApi::TearDown() {
   on_port_connect_.Release();
   on_port_disconnect_.Release();
   on_port_message_.Release();
+
+  // Make sure all our open and in-progress ports are closed.
+  if (messaging_provider_ != NULL)
+    messaging_provider_->CloseAll(this);
   messaging_provider_.Release();
 
   return S_OK;
@@ -73,6 +77,8 @@ STDMETHODIMP ContentScriptNativeApi::OpenChannelToExtension(BSTR source_id,
                                                             long* port_id) {
   // TODO(siggi@chromium.org): handle connecting to other extensions.
   // TODO(siggi@chromium.org): check for the correct source_id here.
+  DCHECK(messaging_provider_ != NULL);
+
   if (0 != wcscmp(com::ToString(source_id), com::ToString(target_id)))
     return E_UNEXPECTED;
 
@@ -130,7 +136,9 @@ STDMETHODIMP ContentScriptNativeApi::PortRelease(long port_id) {
 }
 
 STDMETHODIMP ContentScriptNativeApi::PostMessage(long port_id, BSTR msg) {
-  LocalPortMap::iterator it(local_ports_.find(port_id));
+    LocalPortMap::iterator it(local_ports_.find(port_id));
+  DCHECK(messaging_provider_ != NULL);
+
   // TODO(siggi@chromium.org): should I expect to get messages to
   // defunct port ids?
   DCHECK(it != local_ports_.end());
@@ -189,6 +197,13 @@ STDMETHODIMP ContentScriptNativeApi::put_onPortMessage(IDispatch* callback) {
 
 void ContentScriptNativeApi::OnChannelOpened(int cookie,
                                              int port_id) {
+  // We've observed post-teardown notifications.
+  // TODO(siggi): These shouldn't occur, figure out why this is happening.
+  if (messaging_provider_ == NULL) {
+    LOG(ERROR) << "Received OnChannelOpened after teardown";
+    return;
+  }
+
   // The cookie is our local port ID.
   LocalPortId local_id = cookie;
   LocalPortMap::iterator it(local_ports_.find(local_id));
@@ -203,7 +218,7 @@ void ContentScriptNativeApi::OnChannelOpened(int cookie,
   remote_to_local_port_id_[port.remote_id] = port.local_id;
 
   // Flush pending messages on this port.
-  if (port.pending_messages.size() > 0) {
+  if (!port.pending_messages.empty()) {
     MessageList::iterator it(port.pending_messages.begin());
     MessageList::iterator end(port.pending_messages.end());
 
diff --git a/ceee/ie/plugin/scripting/content_script_native_api_unittest.cc b/ceee/ie/plugin/scripting/content_script_native_api_unittest.cc
index 48b6213..ac24ec9 100644
--- a/ceee/ie/plugin/scripting/content_script_native_api_unittest.cc
+++ b/ceee/ie/plugin/scripting/content_script_native_api_unittest.cc
@@ -52,6 +52,7 @@ class MockIExtensionPortMessagingProvider
 
 class TestingContentScriptNativeApi
     : public ContentScriptNativeApi,
+      public InstanceCountMixin<TestingContentScriptNativeApi>,
       public InitializingCoClass<TestingContentScriptNativeApi> {
  public:
   // Disambiguate.
@@ -204,6 +205,9 @@ TEST_F(ContentScriptNativeApiTest, OnPostMessage) {
 
   function_->ExpectInvoke(DISPID_VALUE, kWideMsg2, local_port_id2);
   api_->OnPostMessage(kRemotePortId2, kMsg2);
+
+  EXPECT_CALL(*mock_provider, CloseAll(api_));
+  api_->TearDown();
 }
 
 }  // namespace