Make commands consistent across security interstitials

chrome/browser/ssl/captive_portal_blocking_page.cc

 // Copyright 2014 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/ssl/captive_portal_blocking_page.h"
 
 #include "base/i18n/rtl.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/captive_portal/captive_portal_tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/captive_portal/captive_portal_detector.h"
 #include "components/wifi/wifi_service.h"
 #include "content/public/browser/web_contents.h"
 #include "grit/generated_resources.h"
 #include "net/base/net_util.h"
 #include "net/base/network_change_notifier.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if !defined(ENABLE_CAPTIVE_PORTAL_DETECTION)
 #error This file must be built with ENABLE_CAPTIVE_PORTAL_DETECTION flag.
 #endif
 
 namespace {
 
 // Events for UMA.
 enum CaptivePortalBlockingPageEvent {
   SHOW_ALL,
   OPEN_LOGIN_PAGE,
   CAPTIVE_PORTAL_BLOCKING_PAGE_EVENT_COUNT
 };
 
-const char kOpenLoginPageCommand[] = "openLoginPage";
-
 void RecordUMA(CaptivePortalBlockingPageEvent event) {
   UMA_HISTOGRAM_ENUMERATION("interstitial.captive_portal",
                             event,
                             CAPTIVE_PORTAL_BLOCKING_PAGE_EVENT_COUNT);
 }
 
 class ConnectionInfoDelegate : public CaptivePortalBlockingPage::Delegate {
  public:
   ConnectionInfoDelegate() {}
   ~ConnectionInfoDelegate() override {}
@@ skipping 136 matching lines @@
   }
 
   // Fill the empty strings to avoid getting debug warnings.
   load_time_data->SetString("openDetails", base::string16());
   load_time_data->SetString("closeDetails", base::string16());
   load_time_data->SetString("explanationParagraph", base::string16());
   load_time_data->SetString("finalParagraph", base::string16());
 }
 
 void CaptivePortalBlockingPage::CommandReceived(const std::string& command) {
-  // The response has quotes around it.
-  if (command == std::string("\"") + kOpenLoginPageCommand + "\"") {
+  int cmd = 0;
+  bool retval = base::StringToInt(command, &cmd);

[meacer, 2015/03/06 18:29:22]

|command| has quotes around it (e.g. "1"), need to remove them before passing it
base::StringToInt. And since all security interstitials have to do this, you
could have a helper method in SecurityInterstitialPage for it.

[meacer, 2015/03/07 01:00:04]

Correction: |command| shouldn't normally have quotes around it as you said, when
sent from the interstitial. But Captive portal and SB tests call
|WaitForRenderFrameReady()| which sends out an extra "pageLoadComplete" message
when the interstitial fully loads:
https://code.google.com/p/chromium/codesearch#chromium/src/content/public/tes...

So you'll need to ignore that message explicitly.

[felt, 2015/03/07 04:02:39]

Done.

+  DCHECK(retval);
+
+  if (cmd == CMD_OPEN_LOGIN) {
     RecordUMA(OPEN_LOGIN_PAGE);
     CaptivePortalTabHelper::OpenLoginTabForWebContents(web_contents(), true);
   }
 }