Fix build by adding a guard to exclude glib code.

Fix build by adding a guard to exclude glib code.

Chromium started to use glib to parse .desktop file:
https://codereview.chromium.org/7000018

However, use_glib is not added when use_ozone is defined,
which causes a build error.
https://codereview.chromium.org/249583003

BUG=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269644

[joone, 2014-05-08 08:09:24]

Could you review this patch?

[Evan Martin, 2014-05-08 17:32:12]

I haven't worked on Chrome in two years.  You probably should find a
different reviewer.  Sorry!


On Thu, May 8, 2014 at 1:09 AM, <joone.hur@intel.com> wrote:

> Reviewers: Evan Martin,
>
> Message:
> Could you review this patch?
>
> Description:
> Fix build by adding a guard to exclude glib code.
>
> Chromium started to use glib to parse .desktop file:
> https://codereview.chromium.org/7000018
>
> However, use_glib is not added when use_ozone is defined,
> which causes a build error.
> https://codereview.chromium.org/249583003
>
> BUG=none
>
> Please review this at https://codereview.chromium.org/276673002/
>
> SVN Base: https://chromium.googlesource.com/chromium/src.git@master
>
> Affected files (+19, -0 lines):
>   M 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 b2b111e989b3ba906f913ea6b644f21ba3a40be3..
> dd7d752a7ebb6e845607a033f03a16e6ef529371 100644
> --- a/chrome/browser/shell_integration_linux.cc
> +++ b/chrome/browser/shell_integration_linux.cc
> @@ -5,7 +5,11 @@
>  #include "chrome/browser/shell_integration_linux.h"
>
>  #include <fcntl.h>
> +
> +#if defined(USE_GLIB)
>  #include <glib.h>
> +#endif
> +
>  #include <stdlib.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
> @@ -444,6 +448,10 @@ ShellIntegration::DefaultWebClientState
> GetIsDefaultWebClient(
>  // Get the value of NoDisplay from the [Desktop Entry] section of a
> .desktop
>  // file, given in |shortcut_contents|. If the key is not found, returns
> false.
>  bool GetNoDisplayFromDesktopFile(const std::string& shortcut_contents) {
> +#if defined(USE_OZONE)
> +  NOTIMPLEMENTED();
> +  return false;
> +#else
>    // An empty file causes a crash with glib <= 2.32, so special case here.
>    if (shortcut_contents.empty())
>      return false;
> @@ -472,6 +480,7 @@ bool GetNoDisplayFromDesktopFile(const std::string&
> shortcut_contents) {
>
>    g_key_file_free(key_file);
>    return nodisplay;
> +#endif
>  }
>
>  // Gets the path to the Chrome executable or wrapper script.
> @@ -778,6 +787,10 @@ std::string GetDesktopFileContentsForCommand(
>      const std::string& icon_name,
>      const std::string& categories,
>      bool no_display) {
> +#if defined(USE_OZONE)
> +  NOTIMPLEMENTED();
> +  return std::string("");
> +#else
>    // Although not required by the spec, Nautilus on Ubuntu Karmic creates
> its
>    // launchers with an xdg-open shebang. Follow that convention.
>    std::string output_buffer = std::string(kXdgOpenShebang) + "\n";
> @@ -844,10 +857,15 @@ std::string GetDesktopFileContentsForCommand(
>
>    g_key_file_free(key_file);
>    return output_buffer;
> +#endif
>  }
>
>  std::string GetDirectoryFileContents(const base::string16& title,
>                                       const std::string& icon_name) {
> +#if defined(USE_OZONE)
> +  NOTIMPLEMENTED();
> +  return std::string("");
> +#else
>    // See http://standards.freedesktop.org/desktop-entry-spec/latest/
>    GKeyFile* key_file = g_key_file_new();
>
> @@ -879,6 +897,7 @@ std::string GetDirectoryFileContents(const
> base::string16& title,
>
>    g_key_file_free(key_file);
>    return output_buffer;
> +#endif
>  }
>
>  bool CreateDesktopShortcut(
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[joone, 2014-05-08 18:37:39]

Hi Elliot,
Could you review my patch?

[Elliot Glaysher, 2014-05-08 19:28:33]

https://codereview.chromium.org/276673002/diff/1/chrome/browser/shell_integra...
File chrome/browser/shell_integration_linux.cc (right):

https://codereview.chromium.org/276673002/diff/1/chrome/browser/shell_integra...
chrome/browser/shell_integration_linux.cc:451: #if defined(USE_OZONE)
Because you're checking USE_GLIB in the header at the top of this file, this
should be !defined(USE_GLIB). (Or USE_GLIB with a switch of the
implementations.)

Same with the rest. Please be consistent with ifdefs.

[joone, 2014-05-09 18:06:30]

On 2014/05/08 19:28:33, Elliot Glaysher wrote:
>
https://codereview.chromium.org/276673002/diff/1/chrome/browser/shell_integra...
> File chrome/browser/shell_integration_linux.cc (right):
> 
>
https://codereview.chromium.org/276673002/diff/1/chrome/browser/shell_integra...
> chrome/browser/shell_integration_linux.cc:451: #if defined(USE_OZONE)
> Because you're checking USE_GLIB in the header at the top of this file, this
> should be !defined(USE_GLIB). (Or USE_GLIB with a switch of the
> implementations.)
> 
> Same with the rest. Please be consistent with ifdefs.

Ok, thanks for the review.
I've update the patch.

[Elliot Glaysher, 2014-05-09 18:27:12]

lgtm

[joone, 2014-05-09 20:29:29]

The CQ bit was checked by joone.hur@intel.com

[commit-bot: I haz the power, 2014-05-09 20:32:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/joone.hur@intel.com/276673002/20001

[commit-bot: I haz the power, 2014-05-09 22:33:34]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-05-11 00:13:58]

Change committed as 269644

[Lei Zhang, 2014-05-23 08:48:22]

https://codereview.chromium.org/276673002/diff/20001/chrome/browser/shell_int...
File chrome/browser/shell_integration_linux.cc (right):

https://codereview.chromium.org/276673002/diff/20001/chrome/browser/shell_int...
chrome/browser/shell_integration_linux.cc:859: return std::string("");
nit: std::string()