Move remaining about: handlers to webUI.

Move remaining about: handlers to webUI.

browser_about_handler.cc still exists to rewrite old about: URLs, but the
actual handling now takes place under webui.  Thus BINDINGS_POLICY_WEB_UI
is applied to these as well.

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

[Tom Sepez, 2011-11-21 22:33:25]

Here's a first cut at this.  Please take a look.

[Tom Sepez, 2011-11-22 00:00:25]

RenderViewHostManagerTest.NonWebUIChromeURLs flunking

Larger question is whether there is a good reason for any of these to
be non-webui pages.

Test explanation:

// Tests that chrome: URLs
 that are not Web UI pages do not get grouped into     
// Web UI renderers, even if --process-per-tab is enabled.  In that mode, we    

// still swap processes if ShouldSwapProcessesForNavigation is true.            

// Regression test for bug 46290.                                               

TEST_F(RenderViewHostManagerTest, NonWebUIChromeURLs)

[Evan Stade, 2011-11-22 02:27:44]

code lgtm, Maybe Charlie has insight into your larger question

http://codereview.chromium.org/8623001/diff/10/chrome/browser/ui/webui/chrome...
File chrome/browser/ui/webui/chrome_web_ui_factory.cc (right):

http://codereview.chromium.org/8623001/diff/10/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chrome_web_ui_factory.cc:289: if (url.host() ==
chrome::kChromeUITCMallocHost)
nit: I would place these macro-guarded checks in the same block as the common
ones

[Charlie Reis, 2011-11-22 17:47:02]

On 2011/11/22 00:00:25, Tom Sepez wrote:
> RenderViewHostManagerTest.NonWebUIChromeURLs flunking
> 
> Larger question is whether there is a good reason for any of these to
> be non-webui pages.
> 
> Test explanation:
> 
> // Tests that chrome: URLs
>  that are not Web UI pages do not get grouped into     
> // Web UI renderers, even if --process-per-tab is enabled.  In that mode, we  
 
> 
> // still swap processes if ShouldSwapProcessesForNavigation is true.          
 
> 
> // Regression test for bug 46290.                                             
 
> 
> TEST_F(RenderViewHostManagerTest, NonWebUIChromeURLs)

Not sure if there's a reason to keep any of the about: URLs as non-WebUI, but I
also don't have access to this bug to see why you want to change them to WebUI. 
(I have some guesses, though.)

The only case that comes to mind is URLs like about:hang, about:shorthang, and
about:crash that must load in the current page.  If those still work, I don't
know of a reason to keep the others as non-WebUI.

[tsepez (do not use), 2011-11-22 18:10:09]

Charlie, I've cc'd you on crbug.com/104466 and crbug.com/104809.

On Tue, Nov 22, 2011 at 9:47 AM, <creis@chromium.org> wrote:

> On 2011/11/22 00:00:25, Tom Sepez wrote:
>
>> RenderViewHostManagerTest.**NonWebUIChromeURLs flunking
>>
>
>  Larger question is whether there is a good reason for any of these to
>> be non-webui pages.
>>
>
>  Test explanation:
>>
>
>  // Tests that chrome: URLs
>>  that are not Web UI pages do not get grouped into
>> // Web UI renderers, even if --process-per-tab is enabled.  In that mode,
>> we
>>
>
>
>  // still swap processes if ShouldSwapProcessesForNavigati**on is true.
>>
>
>
>  // Regression test for bug 46290.
>>
>
>
>  TEST_F(**RenderViewHostManagerTest, NonWebUIChromeURLs)
>>
>
> Not sure if there's a reason to keep any of the about: URLs as non-WebUI,
> but I
> also don't have access to this bug to see why you want to change them to
> WebUI.
> (I have some guesses, though.)
>
> The only case that comes to mind is URLs like about:hang, about:shorthang,
> and
> about:crash that must load in the current page.  If those still work, I
> don't
> know of a reason to keep the others as non-WebUI.
>
>
http://codereview.chromium.**org/8623001/<http://codereview.chromium.org/8623...
>

[Tom Sepez, 2011-11-22 18:34:01]

http://codereview.chromium.org/8623001/diff/10/chrome/browser/ui/webui/chrome...
File chrome/browser/ui/webui/chrome_web_ui_factory.cc (right):

http://codereview.chromium.org/8623001/diff/10/chrome/browser/ui/webui/chrome...
chrome/browser/ui/webui/chrome_web_ui_factory.cc:289: if (url.host() ==
chrome::kChromeUITCMallocHost)
Will do.  My personal preference has always been to do these at statement-level
boundaries where possible, but sure.

[Tom Sepez, 2011-11-22 21:31:21]

Please re-review. Thanks.

[Charlie Reis, 2011-11-22 23:38:39]

Changes to RenderViewHostManagerTest LGTM.  I'll let Evan review the code
changes.

[Evan Stade, 2011-11-23 22:01:09]

lgtm

[commit-bot: I haz the power, 2011-11-30 22:13:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tsepez@chromium.org/8623001/48001

[commit-bot: I haz the power, 2011-11-30 22:13:45]

Can't apply patch for file chrome/browser/ui/webui/about_ui.cc.
While running patch -p0 --forward --force;
patching file chrome/browser/ui/webui/about_ui.cc
Hunk #1 FAILED at 2.
Hunk #2 FAILED at 27.
Hunk #3 FAILED at 49.
Hunk #4 FAILED at 90.
Hunk #5 FAILED at 240.
Hunk #6 FAILED at 250.
Hunk #7 FAILED at 264.
Hunk #8 FAILED at 274.
Hunk #9 FAILED at 285.
Hunk #10 FAILED at 302.
Hunk #11 FAILED at 311.
Hunk #12 FAILED at 371.
Hunk #13 FAILED at 746.
Hunk #14 FAILED at 789.
Hunk #15 FAILED at 870.
Hunk #16 FAILED at 1229.
Hunk #17 FAILED at 1277.
Hunk #18 FAILED at 1373.
Hunk #19 FAILED at 1400.
Hunk #20 FAILED at 1483.
Hunk #21 FAILED at 1499.
21 out of 21 hunks FAILED -- saving rejects to file
chrome/browser/ui/webui/about_ui.cc.rej