Replace most users of BrowserList::GetLastActive in feedback ui with more appropriate functions.

chrome/browser/ui/webui/feedback_ui.cc

 // 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.
 
 #include "chrome/browser/ui/webui/feedback_ui.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 339 matching lines @@
         ReplaceFirstSubstringAfterOffset(&timestamp_,
                                          0,
                                          kTimestampParameter,
                                          "");
         break;
       }
     }
   }
 #endif
 
+  // TODO(beng): Replace GetLastActive with a more specific method of locating
+  //             the target contents.
   Browser* browser = BrowserList::GetLastActive();
   // Sanity checks.
   if (((index == 0) && (index_str != "0")) || !browser ||
       index >= browser->tab_count()) {
     return false;
   }
 
   WebContents* target_tab =
       browser->GetTabContentsWrapperAt(index)->web_contents();
   if (target_tab) {
     target_tab_url_ = target_tab->GetURL().spec();
   }
 
   // Setup the screenshot source after we've verified input is legit.
   SetupScreenshotsSource();
 
   return true;
 }
 
 void FeedbackHandler::RegisterMessages() {
   SetupScreenshotsSource();

[Evan Stade, 2012/05/22 18:46:11]

I have no idea why this is called twice

[Ben Goodger (Google), 2012/05/22 19:21:27]

Initial inspection seems to suggest that via Init(), it is only called if the
tab index is valid. It won't be called if it's invalid, but even if it is
invalid, construction of the FeedbackUI continues (false is passed to
CreateFeedbackUIHTMLSource instead). RegisterMessages() appears to be some sort
of framework function, which will probably end up being called later, so it
seems this code has another stab at initializing the source if it wasn't done
already.

It could be that we want this setup done regardless of whether or not the index
supplied is valid. I don't know any of this code well enough to say what
expectation is/what's the right thing to do.

 
   web_ui()->RegisterMessageCallback("getDialogDefaults",
       base::Bind(&FeedbackHandler::HandleGetDialogDefaults,
                  base::Unretained(this)));
   web_ui()->RegisterMessageCallback("refreshCurrentScreenshot",
       base::Bind(&FeedbackHandler::HandleRefreshCurrentScreenshot,
                  base::Unretained(this)));
 #if defined(OS_CHROMEOS)
   web_ui()->RegisterMessageCallback("refreshSavedScreenshots",
       base::Bind(&FeedbackHandler::HandleRefreshSavedScreenshots,
@@ skipping 149 matching lines @@
   // function, close our feedback tab anyway, we have no more use for it.
   CloseFeedbackTab();
 }
 
 void FeedbackHandler::HandleCancel(const ListValue*) {
   CloseFeedbackTab();
 }
 
 void FeedbackHandler::HandleOpenSystemTab(const ListValue* args) {
 #if defined(OS_CHROMEOS)
-  Browser* last_active = BrowserList::GetLastActive();
-  last_active->OpenURL(
+  web_ui()->GetWebContents()->GetDelegate()->OpenURLFromTab(
+      web_ui()->GetWebContents(),
       content::OpenURLParams(GURL(chrome::kChromeUISystemInfoURL),
                              content::Referrer(),
                              NEW_FOREGROUND_TAB,
                              content::PAGE_TRANSITION_LINK,
                              false));
-  last_active->window()->Activate();
 #endif
 }
 
 void FeedbackHandler::CancelFeedbackCollection() {
 #if defined(OS_CHROMEOS)
   if (syslogs_handle_ != 0) {
     chromeos::system::SyslogsProvider* provider =
         chromeos::system::SyslogsProvider::GetInstance();
     if (provider && syslogs_consumer_.HasPendingRequests())
       provider->CancelRequest(syslogs_handle_);
   }
 #endif
 }
 
 void FeedbackHandler::CloseFeedbackTab() {
   ClobberScreenshotsSource();
-
-  Browser* browser = BrowserList::GetLastActive();
-  if (browser) {
-    browser->CloseTabContents(tab_);
-  } else {
-    LOG(FATAL) << "Failed to get last active browser.";
-  }
+  tab_->GetDelegate()->CloseContents(tab_);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 //
 // FeedbackUI
 //
 ////////////////////////////////////////////////////////////////////////////////
 FeedbackUI::FeedbackUI(content::WebUI* web_ui)
     : WebDialogUI(web_ui) {
   FeedbackHandler* handler = new FeedbackHandler(web_ui->GetWebContents());
@@ skipping 28 matching lines @@
   size_t sort_size = std::min(max_saved, screenshot_filepaths.size());
   std::partial_sort(screenshot_filepaths.begin(),
                     screenshot_filepaths.begin() + sort_size,
                     screenshot_filepaths.end(),
                     ScreenshotTimestampComp);
   for (size_t i = 0; i < sort_size; ++i)
     saved_screenshots->push_back(std::string(kSavedScreenshotsUrl) +
                                    screenshot_filepaths[i]);
 }
 #endif