re-use WebUIs

chrome/browser/ui/webui/help/help_handler.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/help/help_handler.h"
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 167 matching lines @@
 
   localized_strings->SetString("userAgentInfo",
                                content::GetUserAgent(GURL()));
 
   CommandLine::StringType command_line =
       CommandLine::ForCurrentProcess()->GetCommandLineString();
   localized_strings->SetString("commandLineInfo", command_line);
 }
 
 void HelpHandler::RegisterMessages() {
+  registrar_.Add(this, chrome::NOTIFICATION_UPGRADE_RECOMMENDED,
+                 content::NotificationService::AllSources());

[stuartmorgan, 2012/04/24 09:12:20]

Why this change in ordering? If it's important, it should be commented.

[Evan Stade, 2012/04/24 17:52:40]

change in ordering? it's now in a different function. I can comment why, but the
reason for this (and many other changes) is that we may get OnPageLoaded
multiple times, and registering for the same notification multiple times =>
DCHECK.

(I did it this way instead of adding an |observing_| member because it's rare
that an upgrade recommendation notification would come through, so the page
loading will seldom lose the race against upgrade notification. In the other
cases, such as GpuInternals, the race was happening more often, causing lots of
console spew.)

[stuartmorgan, 2012/04/24 19:20:03]

Sorry, totally spaced on the elided lines in the review tool, and thought you
just moved it to the start of the same function.

+
   web_ui()->RegisterMessageCallback("onPageLoaded",
       base::Bind(&HelpHandler::OnPageLoaded, base::Unretained(this)));
   web_ui()->RegisterMessageCallback("relaunchNow",
       base::Bind(&HelpHandler::RelaunchNow, base::Unretained(this)));
   web_ui()->RegisterMessageCallback("openFeedbackDialog",
       base::Bind(&HelpHandler::OpenFeedbackDialog, base::Unretained(this)));
   web_ui()->RegisterMessageCallback("openHelpPage",
       base::Bind(&HelpHandler::OpenHelpPage, base::Unretained(this)));
 #if defined(OS_CHROMEOS)
   web_ui()->RegisterMessageCallback("setReleaseTrack",
@@ skipping 43 matching lines @@
       base::Bind(&HelpHandler::SetUpdateStatus, base::Unretained(this))
 #if defined(OS_MACOSX)
       , base::Bind(&HelpHandler::SetPromotionState, base::Unretained(this))
 #endif
       );
 
 #if defined(OS_CHROMEOS)
   version_updater_->GetReleaseChannel(
       base::Bind(&HelpHandler::OnReleaseChannel, base::Unretained(this)));
 #endif
-
-  registrar_.Add(this, chrome::NOTIFICATION_UPGRADE_RECOMMENDED,
-                 content::NotificationService::AllSources());
 }
 
 #if defined(OS_MACOSX)
 void HelpHandler::PromoteUpdater(const ListValue* args) {
   version_updater_->PromoteUpdater();
 }
 #endif
 
 void HelpHandler::RelaunchNow(const ListValue* args) {
   DCHECK(args->empty());
@@ skipping 110 matching lines @@
   web_ui()->CallJavascriptFunction("help.HelpPage.setOSFirmware",
                                    *firmware_string);
 }
 
 void HelpHandler::OnReleaseChannel(const std::string& channel) {
   scoped_ptr<Value> channel_string(Value::CreateStringValue(channel));
   web_ui()->CallJavascriptFunction(
       "help.HelpPage.updateSelectedChannel", *channel_string);
 }
 #endif // defined(OS_CHROMEOS)