xdg-settings: add secondary check in GetIsDefaultWebClient

chrome/browser/shell_integration_linux.cc

Index: chrome/browser/shell_integration_linux.cc
diff --git a/chrome/browser/shell_integration_linux.cc b/chrome/browser/shell_integration_linux.cc
index 57072b8a9a67f0e32e99a7b504e94fb9fd6a96a6..50a2b24bffdbbc00929333d5af0c3b1c822747bd 100644
--- a/chrome/browser/shell_integration_linux.cc
+++ b/chrome/browser/shell_integration_linux.cc
@@ -164,27 +164,26 @@ bool SetDefaultWebClient(const std::string& protocol) {
 #endif
 }
 
-// If |protocol| is empty this function checks if Chrome is the default browser,
-// otherwise it checks if Chrome is the default handler application for
-// |protocol|.
-DefaultWebClientState GetIsDefaultWebClient(const std::string& protocol) {
-#if defined(OS_CHROMEOS)
-  return UNKNOWN_DEFAULT;
-#else
-  base::ThreadRestrictions::AssertIOAllowed();
-
-  std::unique_ptr<base::Environment> env(base::Environment::Create());
-
+#if !defined(OS_CHROMEOS)
+// If |check| is true, returns the output of "xdg-settings check {property}
+// *.desktop", otherwise returns the output of "xdg-settings get {property}",
+// where property is "default-web-browser" if |protocol| is empty or
+// "default-url-scheme-handler |protocol|" otherwise.  Returns "" if
+// xdg-settings fails for any reason.
+std::string GetXdgSettingsOutput(bool check,
+                                 const std::string& protocol,
+                                 base::Environment* env) {
   std::vector<std::string> argv;
   argv.push_back(kXdgSettings);
-  argv.push_back("check");
+  argv.push_back(check ? "check" : "get");
   if (protocol.empty()) {
     argv.push_back(kXdgSettingsDefaultBrowser);
   } else {
     argv.push_back(kXdgSettingsDefaultSchemeHandler);
     argv.push_back(protocol);
   }
-  argv.push_back(shell_integration_linux::GetDesktopName(env.get()));
+  if (check)
+    argv.push_back(shell_integration_linux::GetDesktopName(env));
 
   std::string reply;
   int success_code;
@@ -199,13 +198,46 @@ DefaultWebClientState GetIsDefaultWebClient(const std::string& protocol) {
 
   if (!ran_ok || success_code != EXIT_SUCCESS) {
     // xdg-settings failed: we can't determine or set the default browser.

[Lei Zhang, 2017/03/10 02:04:45]

Move this comment down to line 239?

[Tom (Use chromium acct), 2017/03/10 02:25:45]

Done.

-    return UNKNOWN_DEFAULT;
+    return std::string();
   }
 
-  // Allow any reply that starts with "yes".
-  return base::StartsWith(reply, "yes", base::CompareCase::SENSITIVE)
-             ? IS_DEFAULT
-             : NOT_DEFAULT;
+  return reply;
+}
+#endif
+
+// If |protocol| is empty this function checks if Chrome is the default browser,
+// otherwise it checks if Chrome is the default handler application for
+// |protocol|.
+DefaultWebClientState GetIsDefaultWebClient(const std::string& protocol) {

[Lei Zhang, 2017/03/10 02:04:45]

Can we remove this to just GetDefaultWebClient() while we are here?

[Tom (Use chromium acct), 2017/03/10 02:25:45]

Done.

+#if defined(OS_CHROMEOS)
+  return UNKNOWN_DEFAULT;
+#else
+  base::ThreadRestrictions::AssertIOAllowed();
+
+  std::unique_ptr<base::Environment> env(base::Environment::Create());
+  std::string xdg_is_default = GetXdgSettingsOutput(true, protocol, env.get());
+  if (base::StartsWith(xdg_is_default, "yes", base::CompareCase::SENSITIVE)) {
+    return IS_DEFAULT;
+  } else if (base::StartsWith(xdg_is_default, "no",

[Lei Zhang, 2017/03/10 02:04:45]

No need for else-if after a return.

[Tom (Use chromium acct), 2017/03/10 02:25:45]

So you mean to change it to just 'if'?
Done.

+                              base::CompareCase::SENSITIVE)) {
+    // An output of "no" does not necessarily mean Chrom[e,ium] is not the
+    // default.  According to the xdg-settings man page, this can happen when
+    // "only some of the underlying settings actually reflect that value". Don't
+    // return NOT_DEFAULT unless we're sure, or else an annoying "Chrome is not
+    // your default browser" banner will appear on every launch
+    // (crbug.com/578888).

[Lei Zhang, 2017/03/10 02:04:45]

Add https:// so code search can auto-linkify.

[Tom (Use chromium acct), 2017/03/10 02:25:45]

Done.

+    if (base::StartsWith(GetXdgSettingsOutput(false, protocol, env.get()),
+                         shell_integration_linux::GetDesktopName(env.get()),
+                         base::CompareCase::SENSITIVE)) {
+      // This is the odd case where 'xdg-settings check' said that Chrome wasn't
+      // the default, but 'xdg-settings get' returned Chrome as the default.
+      return UNKNOWN_DEFAULT;
+    }
+    // xdg-settings says the default is non-Chrome, and is self-consistent.
+    return NOT_DEFAULT;
+  }
+
+  return UNKNOWN_DEFAULT;
 #endif
 }