Fixes minor nits in Instant.

Fixes minor nits in Instant.

Perfcrastination:
- Adhere to the style guide with respect to DCHECKs: Either remove them
  if they are not "must be true", or remove the "if (!...)" handlers.
- Disallow the default constructor in most places.
- Let the unload handler live across Instant pref changes (so that tabs
  are not prematurely destroyed).
- Delay-destruct objects whose methods are callers are on the stack.

BUG=none
[email protected]
TEST=none


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

[sreeram, 2012-09-27 22:44:09]

Please review.

[sky, 2012-09-27 23:51:11]

LGTM

http://codereview.chromium.org/10989062/diff/2001/chrome/browser/ui/browser_i...
File chrome/browser/ui/browser_instant_controller.cc (right):

http://codereview.chromium.org/10989062/diff/2001/chrome/browser/ui/browser_i...
chrome/browser/ui/browser_instant_controller.cc:32: instant_unload_handler_(new
InstantUnloadHandler(browser)) {
Since this is now created in the constructor and never reset there is no need to
use a scoped_ptr.

[sreeram, 2012-09-28 01:11:25]

http://codereview.chromium.org/10989062/diff/2001/chrome/browser/ui/browser_i...
File chrome/browser/ui/browser_instant_controller.cc (right):

http://codereview.chromium.org/10989062/diff/2001/chrome/browser/ui/browser_i...
chrome/browser/ui/browser_instant_controller.cc:32: instant_unload_handler_(new
InstantUnloadHandler(browser)) {
On 2012/09/27 23:51:11, sky wrote:
> Since this is now created in the constructor and never reset there is no need
to
> use a scoped_ptr.

Done.

I was tempted to make instant_unload_handler_ an actual object
(InstantUnloadHandler instant_unload_handler_) instead of a pointer, so I
wouldn't have to explicitly delete it in the destructor, but that would've
required bringing in the full header in the .h file. So I didn't.

[commit-bot: I haz the power, 2012-09-28 06:27:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/10989062/7001

[commit-bot: I haz the power, 2012-09-28 06:27:54]

Failed to apply patch for chrome/browser/instant/instant_controller.cc:
While running patch -p1 --forward --force;
patching file chrome/browser/instant/instant_controller.cc
Hunk #2 FAILED at 394.
Hunk #3 succeeded at 492 (offset 6 lines).
Hunk #4 succeeded at 541 (offset 6 lines).
Hunk #5 succeeded at 550 (offset 6 lines).
Hunk #6 succeeded at 560 (offset 6 lines).
Hunk #7 succeeded at 587 (offset 6 lines).
Hunk #8 succeeded at 619 with fuzz 1 (offset 7 lines).
1 out of 8 hunks FAILED -- saving rejects to file
chrome/browser/instant/instant_controller.cc.rej

[commit-bot: I haz the power, 2012-09-28 10:47:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/[email protected]/10989062/4010

[commit-bot: I haz the power, 2012-09-28 13:30:58]

Change committed as 159247

[sky, 2012-09-28 15:53:01]

On Thu, Sep 27, 2012 at 6:11 PM,  <[email protected]> wrote:
>
>
http://codereview.chromium.org/10989062/diff/2001/chrome/browser/ui/browser_i...
> File chrome/browser/ui/browser_instant_controller.cc (right):
>
>
http://codereview.chromium.org/10989062/diff/2001/chrome/browser/ui/browser_i...
> chrome/browser/ui/browser_instant_controller.cc:32:
> instant_unload_handler_(new InstantUnloadHandler(browser)) {
> On 2012/09/27 23:51:11, sky wrote:
>>
>> Since this is now created in the constructor and never reset there is
>
> no need to
>>
>> use a scoped_ptr.
>
>
> Done.
>
> I was tempted to make instant_unload_handler_ an actual object
> (InstantUnloadHandler instant_unload_handler_) instead of a pointer, so
> I wouldn't have to explicitly delete it in the destructor, but that
> would've required bringing in the full header in the .h file. So I
> didn't.

You should make it a value, not a pointer. From the style guide: "Do
not replace data members with pointers just to avoid an #include."

  -Scott

>
> http://codereview.chromium.org/10989062/

[sreeram, 2012-09-28 15:58:31]

On Fri, Sep 28, 2012 at 8:52 AM, Scott Violet <[email protected]> wrote:
> You should make it a value, not a pointer. From the style guide: "Do
> not replace data members with pointers just to avoid an #include."

Thanks. Will make the change in my next CL.