Do not write a profile path on a BookmarkDragData object...

Do not write a profile path on a BookmarkDragData object since a dragged url (javascript:...) on a web page does not belong to (the profile's) bookmark model.

Currently the following function returns DRAG_NONE and thus the drop operation is not accepted. The reason for the return value is because ops becomes DragDropTypes::DRAG_MOVE (since data.profile_path_ is not empty()) but event.GetSourceOperations() returns DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK.

int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
...
    int ops = data.GetFirstNode(profile_)
        ? DragDropTypes::DRAG_MOVE
        : DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK;
    return bookmark_utils::PreferredDropOperation(event.GetSourceOperations(),
                                                  ops);

BUG=12290
TEST=Open http://delicious.com/help/bookmarklets page and drag the "Bookmark on Delicious" link on "Google Chrome" section to the bookmark bar.


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

[Yusuke Sato, 2009-06-14 16:39:20]

Hi Scott,
Could you review this?

[sky, 2009-06-15 16:01:28]

Can you add a comment describing why you're explicitly using NULL here. LGTM
with a good comment.

[Yusuke Sato, 2009-06-16 02:35:23]

Thanks. I've added some comments. Could you take another look?

--Yusuke

[sky, 2009-06-16 14:36:57]

http://codereview.chromium.org/125111/diff/5/6
File chrome/browser/tab_contents/tab_contents_view_win.cc (right):

http://codereview.chromium.org/125111/diff/5/6#newcode159
Line 159: bm_drag_data.Write(NULL, data);
Might I suggest a slightly different comment:

Pass in NULL as the profile so that the bookmark always adds the url rather than
trying to move an existing url.

[Yusuke Sato, 2009-06-16 16:12:44]

http://codereview.chromium.org/125111/diff/5/6
File chrome/browser/tab_contents/tab_contents_view_win.cc (right):

http://codereview.chromium.org/125111/diff/5/6#newcode159
Line 159: bm_drag_data.Write(NULL, data);
On 2009/06/16 14:36:57, sky wrote:
> Might I suggest a slightly different comment:
> 
> Pass in NULL as the profile so that the bookmark always adds the url rather
than
> trying to move an existing url.

Sure. Fixed.

[sky, 2009-06-16 16:39:22]

LGTM