GTK: Use glib's desktop file parser instead of our hand rolled one.

GTK: Use glib's desktop file parser instead of our hand rolled one.

We need to support .desktop files that are much more complicated than the
simple ones we've used so far. Rip out the our hand rolled parser and use
glib's to manage multi-section desktop files.

We need to be able to build new .desktop files off desktop files that have
multiple groups. Change the unit test so the "real world example" has the
X-Ayatana-Desktop-Shortcuts keys (plus related sections).

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=85059

[Elliot Glaysher, 2011-05-11 01:07:27]

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
File chrome/browser/shell_integration_linux.cc (right):

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
chrome/browser/shell_integration_linux.cc:443: g_key_file_set_string(key_file,
kDesktopEntry, "Exec", final_path.c_str());
This level of quoting is done by the library. (We need to still do
QuoteArgForDesktopFileExec above though or the unit tests complain about
non-equivalent output).

[Elliot Glaysher, 2011-05-11 21:21:05]

ping?

[Evan Martin, 2011-05-11 21:29:06]

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
File chrome/browser/shell_integration_linux.cc (right):

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
chrome/browser/shell_integration_linux.cc:208: // Remove keys that we certainly
don't want.
Can you make this comment describe what this is?  The comment you have here
seems more like a description of what some code later likely does

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
chrome/browser/shell_integration_linux.cc:214: "StartupWMClass",  // read only
on TOOLKIT_GTK
what does this comment mean?

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
chrome/browser/shell_integration_linux.cc:372: //
http://developer.gnome.org/glib/unstable/glib-Key-value-file-parser.html
Oh wow, I had no idea this existed!

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
chrome/browser/shell_integration_linux.cc:412: final_title = url.spec();
Is it possible that url.spec() has these bad characters?

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
chrome/browser/shell_integration_linux.cc:443: g_key_file_set_string(key_file,
kDesktopEntry, "Exec", final_path.c_str());
On 2011/05/11 01:07:27, Elliot Glaysher wrote:
> This level of quoting is done by the library. (We need to still do
> QuoteArgForDesktopFileExec above though or the unit tests complain about
> non-equivalent output).

Are the unit tests correct?  Better to be correct and fix the tests...

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
chrome/browser/shell_integration_linux.cc:458: std::string
output_buffer("#!/usr/bin/env xdg-open");
did you intend to use your kShebang constant?

[Elliot Glaysher, 2011-05-11 21:52:29]

Updated comments.

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
File chrome/browser/shell_integration_linux.cc (right):

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
chrome/browser/shell_integration_linux.cc:412: final_title = url.spec();
On 2011/05/11 21:29:06, Evan Martin wrote:
> Is it possible that url.spec() has these bad characters?

No, but it would if we called invalid_spec(). On debug builds it will dcheck and
return an empty gurl.

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
chrome/browser/shell_integration_linux.cc:443: g_key_file_set_string(key_file,
kDesktopEntry, "Exec", final_path.c_str());
On 2011/05/11 21:29:06, Evan Martin wrote:
> On 2011/05/11 01:07:27, Elliot Glaysher wrote:
> > This level of quoting is done by the library. (We need to still do
> > QuoteArgForDesktopFileExec above though or the unit tests complain about
> > non-equivalent output).
> 
> Are the unit tests correct?  Better to be correct and fix the tests...

Yes. It's just a weird artifact of where gkeyparser escapes stuff. (The unit
tests also call out this escaping behaviour specifically.)

http://codereview.chromium.org/7000018/diff/1/chrome/browser/shell_integratio...
chrome/browser/shell_integration_linux.cc:458: std::string
output_buffer("#!/usr/bin/env xdg-open");
On 2011/05/11 21:29:06, Evan Martin wrote:
> did you intend to use your kShebang constant?

No, because that constant has a newline. Updated though.

[Evan Martin, 2011-05-11 21:54:15]

LGTM

http://codereview.chromium.org/7000018/diff/4002/chrome/browser/shell_integra...
File chrome/browser/shell_integration_linux.cc (right):

http://codereview.chromium.org/7000018/diff/4002/chrome/browser/shell_integra...
chrome/browser/shell_integration_linux.cc:459: std::string output_buffer =
kXdgOpenShebang;
Do you need a newline here?  I guess the tests must cover this.