Enable history and downloads by default, port NewTabUI from DOMUIHost to DOMU...

Enable history and downloads by default, port NewTabUI from DOMUIHost to DOMUI.

NewTabUI is only included on OS(WIN) because DOMUI doesn't appear to have been ported.

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

[Ben Goodger (Google), 2009-02-25 02:57:27]

http://codereview.chromium.org/28104/diff/112/118
File chrome/browser/browser.cc (right):

http://codereview.chromium.org/28104/diff/112/118#newcode963
Line 963: NULL);
nit: indent to (

http://codereview.chromium.org/28104/diff/112/118#newcode976
Line 976: NULL);
ditto

http://codereview.chromium.org/28104/diff/112/131
File chrome/browser/dom_ui/dom_ui_contents.cc (right):

http://codereview.chromium.org/28104/diff/112/131#newcode179
Line 179: return true;
nit: no else after return.

http://codereview.chromium.org/28104/diff/112/131#newcode186
Line 186: return false;
and here

http://codereview.chromium.org/28104/diff/112/131#newcode198
Line 198: return true;
and here

http://codereview.chromium.org/28104/diff/112/131#newcode208
Line 208: if (reload) {
no braces

http://codereview.chromium.org/28104/diff/112/131#newcode256
Line 256: return NULL;
no else after return.

http://codereview.chromium.org/28104/diff/112/126
File chrome/browser/dom_ui/dom_ui_host.h (right):

http://codereview.chromium.org/28104/diff/112/126#newcode5
Line 5: // THIS FILE IS DEPRECATED, USE DOM_UI INSTEAD.
who still uses this?

[Ben Goodger (Google), 2009-02-25 03:05:39]

This seems OK to me.

One request I do have is that you put together a design doc on dev.chromium.org
that briefly sketches out how these DOMUIs work. Just a class diagram explaining
who does what is sufficient.

[pink (ping after 24hrs), 2009-02-25 15:42:00]

I think this should probably work now on POSIX w/out the new_tab_ui ifdefs. If
you want to make those changes and give it a shot on the try server (or tell me
when and i'll apply the patch on mac), let me know.

Mac&Linux are still building dom_ui_host.cc, so you should remove those in a
follow-up CL. File a bug so you don't forget?

http://codereview.chromium.org/28104/diff/112/118
File chrome/browser/browser.cc (right):

http://codereview.chromium.org/28104/diff/112/118#newcode961
Line 961: GURL downloads_url = HistoryUI::GetBaseURL();
why would the history url be put a variable named |downloads_url|? Cut and paste
error?

http://codereview.chromium.org/28104/diff/112/131
File chrome/browser/dom_ui/dom_ui_contents.cc (right):

http://codereview.chromium.org/28104/diff/112/131#newcode15
Line 15: #if defined(OS_WIN)
probably not necessary anymore since new_tab_ui compiles on the trunk.

http://codereview.chromium.org/28104/diff/112/131#newcode244
Line 244: #if defined(OS_WIN)
this would probably work, but if not for some reason, make sure you add a
TODO(PORT) comment and a NOTIMPLEMENTED(); so we can find it later.

http://codereview.chromium.org/28104/diff/112/119
File chrome/browser/safe_browsing/safe_browsing_blocking_page.cc (right):

http://codereview.chromium.org/28104/diff/112/119#newcode27
Line 27: #include "chrome/browser/dom_ui/new_tab_ui.h"
this would probably work on posix.

http://codereview.chromium.org/28104/diff/112/117
File chrome/browser/tab_contents/tab_contents_factory.cc (right):

http://codereview.chromium.org/28104/diff/112/117#newcode19
Line 19: #include "chrome/browser/dom_ui/new_tab_ui.h"
this will probably work on posix.

http://codereview.chromium.org/28104/diff/112/117#newcode68
Line 68: case TAB_CONTENTS_NEW_TAB_UI:
not sure if we're hooked up for this to work, but it might. maybe do this in
stages where if we can get everything landed, we can go in with a round-two and
pull this out of the define?

http://codereview.chromium.org/28104/diff/112/132
File chrome/common/temp_scaffolding_stubs.h (right):

http://codereview.chromium.org/28104/diff/112/132#newcode689
Line 689: class NewTabUI {
might not be necessary.