Fixes navigations chrome-internal: to actually show the NTP....

Fixes navigations chrome-internal: to actually show the NTP.

We now rewrite chrome-internal: (the old NTP URL) to chrome://newtab.  Before, we were not properly showing the NTP for this URL, which caused some problems with further navigations (particularly form submissions) from that tab.

Also, the default kHomePage string is now chrome://newtab instead of the outdated chrome-internal:.  The logic in GeneralPageView is already designed to not show this URL in the preferences dialog.

BUG=6564
TEST=NewTabUITest.ChromeInternalLoadsNTP


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

[Charlie Reis, 2009-11-05 01:30:40]

Here's a first attempt at fixing chrome-internal: (and thus 6564).

I think we should also change Browser::RegisterUserPrefs to use
chrome::kChromeUINewTabURL instead of L"chrome-internal:" on line 1255 of
browser.cc:
  prefs->RegisterStringPref(prefs::kHomePage, L"chrome-internal:");

Does that sound correct?

[Paweł Hajdan Jr., 2009-11-05 06:38:46]

Drive-by.

http://codereview.chromium.org/363019/diff/4001/4003
File chrome/browser/dom_ui/new_tab_ui_uitest.cc (right):

http://codereview.chromium.org/363019/diff/4001/4003#newcode59
Line 59: scoped_refptr<TabProxy> tab = window->GetActiveTab();
Could you tell it to use the first tab instead? It's a more solid automation
call.

[Charlie Reis, 2009-11-05 17:41:01]

Fixed Paweł's suggestion, and made the change to the default kHomePage pref.

Ben, I'm adding you to the review, since this updates a pref that is visible in
the Options.  We were showing "chrome-internal:" in the default home page box,
and now we show nothing.  (GeneralPageView is designed to not display the new
tab page URL if kHomePage is set to it.)  I think that was the intended
behavior, but let me know if it's wrong.

http://codereview.chromium.org/363019/diff/4001/4003
File chrome/browser/dom_ui/new_tab_ui_uitest.cc (right):

http://codereview.chromium.org/363019/diff/4001/4003#newcode59
Line 59: scoped_refptr<TabProxy> tab = window->GetActiveTab();
On 2009/11/05 06:38:46, Paweł Hajdan Jr. wrote:
> Could you tell it to use the first tab instead? It's a more solid automation
> call.

No problem!

[Ben Goodger (Google), 2009-11-05 23:11:18]

So if I clear out the default home page box, it'll open the NTP?

-Ben

On Thu, Nov 5, 2009 at 9:41 AM,  <creis@chromium.org> wrote:
> Fixed Pawe=C5=82's suggestion, and made the change to the default kHomePa=
ge pref.
>
> Ben, I'm adding you to the review, since this updates a pref that is visi=
ble
> in
> the Options. =C2=A0We were showing "chrome-internal:" in the default home=
 page
> box,
> and now we show nothing. =C2=A0(GeneralPageView is designed to not displa=
y the
> new
> tab page URL if kHomePage is set to it.) =C2=A0I think that was the inten=
ded
> behavior, but let me know if it's wrong.
>
>
> http://codereview.chromium.org/363019/diff/4001/4003
> File chrome/browser/dom_ui/new_tab_ui_uitest.cc (right):
>
> http://codereview.chromium.org/363019/diff/4001/4003#newcode59
> Line 59: scoped_refptr<TabProxy> tab =3D window->GetActiveTab();
> On 2009/11/05 06:38:46, Pawe=C5=82 Hajdan Jr. wrote:
>>
>> Could you tell it to use the first tab instead? It's a more solid
>
> automation
>>
>> call.
>
> No problem!
>
> http://codereview.chromium.org/363019
>

[(Do not use) creis at google, 2009-11-05 23:15:55]

By default, NTP is selected and the home page box looks blank.

If you put in a home page and later try to clear it out, it looks like the
home page you previously entered stays in effect.  (This is true both befor=
e
and after my changelist.)

Charlie


On Thu, Nov 5, 2009 at 3:10 PM, Ben Goodger (Google) <ben@chromium.org>wrot=
e:

> So if I clear out the default home page box, it'll open the NTP?
>
> -Ben
>
> On Thu, Nov 5, 2009 at 9:41 AM,  <creis@chromium.org> wrote:
> > Fixed Pawe=C5=82's suggestion, and made the change to the default kHome=
Page
> pref.
> >
> > Ben, I'm adding you to the review, since this updates a pref that is
> visible
> > in
> > the Options.  We were showing "chrome-internal:" in the default home pa=
ge
> > box,
> > and now we show nothing.  (GeneralPageView is designed to not display t=
he
> > new
> > tab page URL if kHomePage is set to it.)  I think that was the intended
> > behavior, but let me know if it's wrong.
> >
> >
> > http://codereview.chromium.org/363019/diff/4001/4003
> > File chrome/browser/dom_ui/new_tab_ui_uitest.cc (right):
> >
> > http://codereview.chromium.org/363019/diff/4001/4003#newcode59
> > Line 59: scoped_refptr<TabProxy> tab =3D window->GetActiveTab();
> > On 2009/11/05 06:38:46, Pawe=C5=82 Hajdan Jr. wrote:
> >>
> >> Could you tell it to use the first tab instead? It's a more solid
> >
> > automation
> >>
> >> call.
> >
> > No problem!
> >
> > http://codereview.chromium.org/363019
> >
>

[Ben Goodger (Google), 2009-11-05 23:18:48]

OK. LGTM then.

On Thu, Nov 5, 2009 at 3:15 PM, Charles Reis <creis@google.com> wrote:
> By default, NTP is selected and the home page box looks blank.
> If you put in a home page and later try to clear it out, it looks like th=
e
> home page you previously entered stays in effect. =C2=A0(This is true bot=
h before
> and after my changelist.)
> Charlie
>
> On Thu, Nov 5, 2009 at 3:10 PM, Ben Goodger (Google) <ben@chromium.org>
> wrote:
>>
>> So if I clear out the default home page box, it'll open the NTP?
>>
>> -Ben
>>
>> On Thu, Nov 5, 2009 at 9:41 AM, =C2=A0<creis@chromium.org> wrote:
>> > Fixed Pawe=C5=82's suggestion, and made the change to the default kHom=
ePage
>> > pref.
>> >
>> > Ben, I'm adding you to the review, since this updates a pref that is
>> > visible
>> > in
>> > the Options. =C2=A0We were showing "chrome-internal:" in the default h=
ome
>> > page
>> > box,
>> > and now we show nothing. =C2=A0(GeneralPageView is designed to not dis=
play
>> > the
>> > new
>> > tab page URL if kHomePage is set to it.) =C2=A0I think that was the in=
tended
>> > behavior, but let me know if it's wrong.
>> >
>> >
>> > http://codereview.chromium.org/363019/diff/4001/4003
>> > File chrome/browser/dom_ui/new_tab_ui_uitest.cc (right):
>> >
>> > http://codereview.chromium.org/363019/diff/4001/4003#newcode59
>> > Line 59: scoped_refptr<TabProxy> tab =3D window->GetActiveTab();
>> > On 2009/11/05 06:38:46, Pawe=C5=82 Hajdan Jr. wrote:
>> >>
>> >> Could you tell it to use the first tab instead? It's a more solid
>> >
>> > automation
>> >>
>> >> call.
>> >
>> > No problem!
>> >
>> > http://codereview.chromium.org/363019
>> >
>
>