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

chrome/browser/browser_init.cc

Index: chrome/browser/browser_init.cc
diff --git a/chrome/browser/browser_init.cc b/chrome/browser/browser_init.cc
index 8dee0dcf1fc6f384347d199263093e7ae2337048..cc4023ddf9c2892eea7da355cf7e9a3ca1c90c32 100644
--- a/chrome/browser/browser_init.cc
+++ b/chrome/browser/browser_init.cc
@@ -200,8 +200,6 @@ class CheckDefaultBrowserTask : public Task {
 
 // 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)
@@ -238,6 +236,41 @@ class SessionCrashedInfoBarDelegate : public ConfirmInfoBarDelegate {
   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);
@@ -587,8 +620,13 @@ Browser* BrowserInit::LaunchWithProfile::OpenURLsInBrowser(
         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
@@ -611,6 +649,26 @@ void BrowserInit::LaunchWithProfile::AddCrashedInfoBarIfNecessary(
   }
 }
 
+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;