Revert "Revert 84829 - Initial implementation of "Synced Bookmarks" folder."

chrome/browser/ui/gtk/bookmarks/bookmark_editor_gtk.cc

 // Copyright (c) 2011 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/ui/gtk/bookmarks/bookmark_editor_gtk.h"
 
 #include <gtk/gtk.h>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
@@ skipping 312 matching lines @@
     tree_store_ = bookmark_utils::MakeFolderTreeStore();
     bookmark_utils::AddToTreeStore(bb_model_, selected_id, tree_store_,
                                    &selected_iter);
     tree_view_ = bookmark_utils::MakeTreeViewForStore(tree_store_);
     gtk_widget_set_size_request(tree_view_, kTreeWidth, kTreeHeight);
     tree_selection_ = gtk_tree_view_get_selection(GTK_TREE_VIEW(tree_view_));
     g_signal_connect(tree_view_, "button-press-event",
                      G_CALLBACK(OnTreeViewButtonPressEventThunk), this);
 
     GtkTreePath* path = NULL;
-    if (selected_id) {
+    if (selected_id && selected_iter.user_data != NULL) {

[Elliot Glaysher, 2011/05/18 23:51:42]

I don't understand this modified line. I don't see where user_data gets set or
why you can rely on this.

[Yaron, 2011/05/19 00:05:14]

It gets set within bookmark_utils::AddToTreeStore which calls
bookmark_utils::AddToTreeStoreAt which copies iter to  selected_iter.
(http://google.com/codesearch/p?hl=en#OAMlx_jo-ck/src/chrome/browser/ui/gtk/bo...)


iter's user_data field gets set in AddSingleNodeToTreeStore, presumably in one
of the gtk calls:
http://google.com/codesearch/p?hl=en#OAMlx_jo-ck/src/chrome/browser/ui/gtk/bo...

The way I discovered this was that before we wouldn't show Synced Bookmarks even
if it had children. So if the current page was bookmarked, I hit the star, and
clicked edit, it would crash on a check for user_data being non-null. Since this
was technically fixed another way (we no longer hide nodes if they have
children), this isn't strictly necessary but I thought it was safer to include
it. I can remove it if you think that's better.

       path = gtk_tree_model_get_path(GTK_TREE_MODEL(tree_store_),
                                      &selected_iter);
     } else {
       // We don't have a selected parent (Probably because we're making a new
       // bookmark). Select the first item in the list.
       path = gtk_tree_path_new_from_string("0");
     }
 
     gtk_tree_view_expand_to_path(GTK_TREE_VIEW(tree_view_), path);
     gtk_tree_selection_select_path(tree_selection_, path);
@@ skipping 231 matching lines @@
       GTK_TREE_MODEL(tree_store_), &new_item_iter);
   gtk_tree_view_expand_to_path(GTK_TREE_VIEW(tree_view_), path);
 
   // Make the folder name editable.
   gtk_tree_view_set_cursor(GTK_TREE_VIEW(tree_view_), path,
       gtk_tree_view_get_column(GTK_TREE_VIEW(tree_view_), 0),
       TRUE);
 
   gtk_tree_path_free(path);
 }