Show an infobar when the user is using an unsupported flag.

chrome/browser/browser_init.cc

 // Copyright (c) 2009 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/browser_init.h"
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/event_recorder.h"
 #include "base/path_service.h"
@@ skipping 182 matching lines @@
     ChromeThread::PostTask(
         ChromeThread::UI, FROM_HERE, new NotifyNotDefaultBrowserTask());
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(CheckDefaultBrowserTask);
 };
 
 // A delegate for the InfoBar shown when the previous session has crashed. The
 // bar deletes itself automatically after it is closed.
-// TODO(timsteele): This delegate can leak when a tab is closed, see
-// http://crbug.com/6520
 class SessionCrashedInfoBarDelegate : public ConfirmInfoBarDelegate {
  public:
   explicit SessionCrashedInfoBarDelegate(TabContents* contents)
       : ConfirmInfoBarDelegate(contents),
         profile_(contents->profile()) {
   }
 
   // Overridden from ConfirmInfoBarDelegate:
   virtual void InfoBarClosed() {
     delete this;
@@ skipping 16 matching lines @@
     return true;
   }
 
  private:
   // The Profile that we restore sessions from.
   Profile* profile_;
 
   DISALLOW_COPY_AND_ASSIGN(SessionCrashedInfoBarDelegate);
 };
 
+// A delegate for the InfoBar shown when you're using unsupported
+// command-line flags. The bar deletes itself automatically after it
+// is closed.
+class BadFlagsInfoBarDelegate : public AlertInfoBarDelegate {
+ public:
+  explicit BadFlagsInfoBarDelegate(TabContents* contents, const char* flag)
+      : AlertInfoBarDelegate(contents), flag_(flag) {
+  }
+
+  virtual bool ShouldExpire(
+      const NavigationController::LoadCommittedDetails& details) const {
+    // Require explicit closure by the user.
+    return false;
+  }
+
+  virtual void InfoBarClosed() {
+    delete this;
+  }
+
+  virtual std::wstring GetMessageText() const {
+    // Since our flag names are in English anyway, I don't think we should

[viettrungluu, 2009/12/11 19:15:51]

Surely people can use flags even if they don't understand English, and they need
the warning even more if they don't understand the flag names.

+    // make this string translatable.
+    return ASCIIToWide(
+        StringPrintf("You are using an unsupported command-line flag: --%s.  "

[viettrungluu, 2009/12/11 19:15:51]

I'm sceptical that there should be two spaces after the period.

[Evan Martin, 2009/12/11 19:18:45]

Religious debate!  :P
I will change it.  Habit.

+                     "Stability and security will suffer.",
+                     flag_));
+  }
+
+ private:
+  // The flag that we're complaining about.
+  const char* flag_;
+
+  DISALLOW_COPY_AND_ASSIGN(BadFlagsInfoBarDelegate);
+};
+
 SessionStartupPref GetSessionStartupPref(const CommandLine& command_line,
                                          Profile* profile) {
   SessionStartupPref pref = SessionStartupPref::GetStartupPref(profile);
   if (command_line.HasSwitch(switches::kRestoreLastSession))
     pref.type = SessionStartupPref::LAST;
   if (command_line.HasSwitch(switches::kIncognito) &&
       pref.type == SessionStartupPref::LAST) {
     // We don't store session information when incognito. If the user has
     // chosen to restore last session and launched incognito, fallback to
     // default launch behavior.
@@ skipping 329 matching lines @@
     // We skip URLs that we'd have to launch an external protocol handler for.
     // This avoids us getting into an infinite loop asking ourselves to open
     // a URL, should the handler be (incorrectly) configured to be us. Anyone
     // asking us to open such a URL should really ask the handler directly.
     if (!process_startup && !URLRequest::IsHandledURL(urls[i]))
       continue;
     TabContents* tab = browser->AddTabWithURL(
         urls[i], GURL(), PageTransition::START_PAGE, (i == 0), -1, false, NULL);
     if (i < static_cast<size_t>(pin_count))
       browser->tabstrip_model()->SetTabPinned(browser->tab_count() - 1, true);
-    if (profile_ && i == 0 && process_startup)
-      AddCrashedInfoBarIfNecessary(tab);
+
+    if (i == 0 && process_startup) {

[viettrungluu, 2009/12/11 19:15:51]

I would slightly prefer if it were "process_startup && i == 0" for reasons one
may be able to work out (there are two).

[Evan Martin, 2009/12/11 19:18:45]

I was matching the existing code, which I think dominates any other reasoning
here (it's not performance-critical).  But I'll change it if you insist.

[viettrungluu, 2009/12/11 19:45:40]

Fair enough. I definitely don't insist.

+      // First tab; attach necessary InfoBars to it.
+      if (profile_)
+        AddCrashedInfoBarIfNecessary(tab);
+      AddBadFlagsInfoBarIfNecessary(tab);
+    }
   }
   browser->window()->Show();
   // TODO(jcampan): http://crbug.com/8123 we should not need to set the initial
   //                focus explicitly.
   browser->GetSelectedTabContents()->view()->SetInitialFocus();
 
   return browser;
 }
 
 void BrowserInit::LaunchWithProfile::AddCrashedInfoBarIfNecessary(
     TabContents* tab) {
   // Assume that if the user is launching incognito they were previously
   // running incognito so that we have nothing to restore from.
   if (!profile_->DidLastSessionExitCleanly() &&
       !profile_->IsOffTheRecord()) {
     // The last session didn't exit cleanly. Show an infobar to the user
     // so that they can restore if they want. The delegate deletes itself when
     // it is closed.
     tab->AddInfoBar(new SessionCrashedInfoBarDelegate(tab));
   }
 }
 
+void BrowserInit::LaunchWithProfile::AddBadFlagsInfoBarIfNecessary(
+    TabContents* tab) {
+  static const char* kBadFlags[] = {
+    switches::kSingleProcess,
+    switches::kNoSandbox,
+    NULL
+  };
+
+  const char* bad_flag = NULL;
+  for (const char** flag = kBadFlags; *flag; ++flag) {
+    if (command_line_.HasSwitch(*flag)) {
+      bad_flag = *flag;

[viettrungluu, 2009/12/11 19:15:51]

I'm a little disappointed that you only warn of at most one flag (which is
evidently fine for the current list of bad flags), but that's okay.

[Evan Martin, 2009/12/11 19:18:45]

You run out of space in the infobar otherwise.

[Mohamed Mansour, 2009/12/11 19:42:35]

This is good that way, because we can put many BadFlagsInfoBar if necessary. One
for each flag. That will surely annoy the user :) which is of course full of
awesomeness.

[viettrungluu, 2009/12/11 19:45:40]

Good point.

+      break;
+    }
+  }
+
+  if (bad_flag)
+    tab->AddInfoBar(new BadFlagsInfoBarDelegate(tab, bad_flag));
+}
+
 std::vector<GURL> BrowserInit::LaunchWithProfile::GetURLsFromCommandLine(
     Profile* profile) {
   std::vector<GURL> urls;
   std::vector<std::wstring> params = command_line_.GetLooseValues();
   for (size_t i = 0; i < params.size(); ++i) {
     std::wstring& value = params[i];
     // Handle Vista way of searching - "? <search-term>"
     if (value.find(L"? ") == 0) {
       const TemplateURL* default_provider =
           profile->GetTemplateURLModel()->GetDefaultSearchProvider();
@@ skipping 228 matching lines @@
   scoped_refptr<AutomationProviderClass> automation =
       new AutomationProviderClass(profile);
   automation->ConnectToChannel(channel_id);
   automation->SetExpectedTabCount(expected_tabs);
 
   AutomationProviderList* list =
       g_browser_process->InitAutomationProviderList();
   DCHECK(list);
   list->AddProvider(automation);
 }