Make history title inclusion safer (createTextNode changes)....

Resolve crash when deleting history by preventing the deleter from being called multiple times. We need to add UI to make what's happening clearer to the user, but this gets us over the hump for now. Also change the history page to queue deletions.Allow history search from the new tab page.Make history title inclusion safer (createTextNode changes).Show starred status on history page.BUG=8214, 8163, 8271, 8284
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=10773

[Finnur, 2009-03-02 23:09:07]

I am not familiar with this JS code and don't have extensive JS experience, but
this seems reasonable.

http://codereview.chromium.org/28308/diff/1024/1028
File chrome/browser/dom_ui/history_ui.cc (right):

http://codereview.chromium.org/28308/diff/1024/1028#newcode213
Line 213: remover_ = NULL;
You leak a remover object here. Convert to scoped ptr?

http://codereview.chromium.org/28308/diff/1024/1028#newcode381
Line 381: return GURL(GetBaseURL().spec() + "/#q=" +
Not sure I follow here. Why change this?

http://codereview.chromium.org/28308/diff/1024/1025
File chrome/browser/resources/history.html (right):

http://codereview.chromium.org/28308/diff/1024/1025#newcode693
Line 693: $('term').focus();
Huh... I didn't know you could do this... Interesting.

http://codereview.chromium.org/28308/diff/1024/1025#newcode739
Line 739: }
Your teeth look fine. You don't need braces.

http://codereview.chromium.org/28308/diff/1024/1025#newcode779
Line 779: deleteQueue = [];
This looks redundant on the face of it. Why is this needed?

[Glen Murphy, 2009-03-03 01:25:11]

Done with the following comments:


> http://codereview.chromium.org/28308/diff/1024/1028#newcode381
> Line 381: return GURL(GetBaseURL().spec() + "/#q=" +
> Not sure I follow here. Why change this?

The URL that was being returned previously was wrong (it was the URL from the
NativeUI).

[Finnur, 2009-03-03 01:32:33]

Looks like you are missing history_ui.h

If the only change to that is the scoped ptr stuff then LGTM.

On 2009/03/03 01:25:11, Glen Murphy wrote:
> Done with the following comments:
> 
> 
> > http://codereview.chromium.org/28308/diff/1024/1028#newcode381
> > Line 381: return GURL(GetBaseURL().spec() + "/#q=" +
> > Not sure I follow here. Why change this?
> 
> The URL that was being returned previously was wrong (it was the URL from the
> NativeUI).