Avoid double-setting of the tool band ID in the CEEE executors manager....

ceee/ie/plugin/toolband/tool_band.cc

 // Copyright (c) 2010 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.
 //
 // IE toolband implementation.
 #include "ceee/ie/plugin/toolband/tool_band.h"
 
 #include <atlsafe.h>
 #include <atlstr.h>
 #include <shlguid.h>
@@ skipping 39 matching lines @@
 _ATL_FUNC_INFO ToolBand::handler_type_idispatch_variantref_ =
     { CC_STDCALL, VT_EMPTY, 2, { VT_DISPATCH, VT_VARIANT | VT_BYREF } };
 
 ToolBand::ToolBand()
     : already_tried_installing_(false),
       own_line_flag_(false),
       already_checked_own_line_flag_(false),
       listening_to_browser_events_(false),
       band_id_(0),
       is_quitting_(false),
+      already_sent_id_to_bho_(false),
       current_width_(kToolBandMinWidth),
       current_height_(kToolBandHeight) {
   TRACE_EVENT_BEGIN("ceee.toolband", this, "");
 }
 
 ToolBand::~ToolBand() {
   TRACE_EVENT_END("ceee.toolband", this, "");
 }
 
 HRESULT ToolBand::FinalConstruct() {
@@ skipping 452 matching lines @@
   // on overinstall when Chrome has already auto-updated.
   //
   // If it turns out no extension is installed, we will handle that
   // error in the OnCfGetEnabledExtensionsComplete callback.
   chrome_frame_->getEnabledExtensions();
 }
 
 STDMETHODIMP_(void) ToolBand::OnCfGetEnabledExtensionsComplete(
     SAFEARRAY* extension_directories) {
   CComSafeArray<BSTR> directories;
   EnsureBhoIsAvailable();  // We will require a working bho to do anything.

[motek., 2010/11/30 22:33:09]

So, what about 'if (SUCCEEDED(EnsureBhoIsAvailable)) { do the ID thing }

[Cindy Lau, 2010/12/01 00:01:36]

The other thing EnsureBhoIsAvailable does is to actually retrieve the handle to
the BHO; if we want to move SendSessionIdToBho outside of it, we'll have to
somehow return the BHO handle from the EnsureBhoIsAvailable call, perhaps by an
output parameter.  I'm not a huge fan of this approach; if the problem is that
EnsureBhoIsAvailable shouldn't return S_FALSE just because SendSessionIdToBho
does, then I will make a change inside EnsureBhoIsAvailable that will make it
return S_OK if SendSessionIdToBho returns S_FALSE.  Does that sound better?

[motek., 2010/12/01 00:13:49]

On 2010/12/01 00:01:36, Cindy Lau wrote:
Yes, my point was only solely about return values (S_FALSE vs. S_OK). What you
suggest makes more sense. Besides, the whole comment was a minor remark to begin
with.

   directories.Attach(extension_directories);  // MUST DETACH BEFORE RETURNING
 
   // TODO(joi@chromium.org) Handle multiple extensions.
   if (directories.GetCount() > 0) {
     // If our extension_path is not a CRX, it MUST be the same as the installed
     // extension path which would be an exploded extension.
     // If you get this DCHECK, you may have changed your registry settings to
     // debug with an exploded extension, but you didn't uninstall the previous
     // extension, either via the Chrome UI or by simply wiping out your
     // profile folder.
@@ skipping 192 matching lines @@
     chrome_frame_internal_.QueryFrom(chrome_frame_);
     if (chrome_frame_internal_) {
       return chrome_frame_internal_->getSessionId(session_id);
     }
   }
   NOTREACHED();
   return E_UNEXPECTED;
 }
 
 HRESULT ToolBand::SendSessionIdToBho(IUnknown* bho) {
+  if (already_sent_id_to_bho_)
+    return S_FALSE;

[motek., 2010/11/30 22:33:09]

I am not sure if the return value of 'Ensure' should depend on how many times
the ID has been sent to the recipient. I realize we don't use the return value
of 'Ensure' (and so you may disregard this comment), but since now something
actually depends on it, we should do it?
I do recall this was your comment to my original CL, so this is a bit ironic,
but...

[Cindy Lau, 2010/12/01 00:01:36]

See my comment above.

   // Now send the tool band's session ID to the BHO.
   ScopedComPtr<ICeeeBho> ceee_bho;
   HRESULT hr = ceee_bho.QueryFrom(bho);
   if (SUCCEEDED(hr)) {
     int session_id = 0;
     hr = GetSessionId(&session_id);
     if (SUCCEEDED(hr)) {
-      return ceee_bho->SetToolBandSessionId(session_id);
+      hr = ceee_bho->SetToolBandSessionId(session_id);
+      if (SUCCEEDED(hr))
+        already_sent_id_to_bho_ = true;
+      return hr;
     }
   }
   return E_FAIL;
 }