NTP4: Fix crash when re-ordering apps.

NTP4: Fix crash when re-ordering apps.

R=csilv@chromium.org
BUG=104059
TEST=Re-order apps, Chrome doesn't crash when a CHECK() fails.


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

[csilv, 2011-11-14 18:32:06]

lgtm

[commit-bot: I haz the power, 2011-11-14 18:32:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dbeam@chromium.org/8533014/1

[Dan Beam, 2011-11-14 18:51:07]

estade,csilv: We might consider changing how we forward calls like this to be
more generic.  Here's the way that I've considered in the past (I think it
didn't land because it was in the first run CL from a month or two ago):

  function func() {
    return localVar.func.apply(localVar, arguments);
  }

  return {
    ...
    func: func,
    ...
  };

This could also simply be:

  return {
    ...
    func: localVar.func.bind(localVar),
    ...
  };

Which I'd prefer.

And I'd also propose if we don't use the function locally in new_tab.js but have
to do argument modification, we might want to just make this an anonymous
function instead of a local, i.e.

  return {
    ...
    func: function() {
      // slice off first arg for some reason
      var args = Array.prototype.slice.call(arguments, 1);
      return localVar.func.apply(localVar, args);
    },
    ...
  };

What do you think?  The .bind() way seems fine for our cases and this kind of
issue shouldn't reoccur this way.

[Evan Stade, 2011-11-14 19:12:50]

I don't think bind would work because at the time the return statement is
executed, newTabView is still undefined.

[commit-bot: I haz the power, 2011-11-14 19:41:44]

Change committed as 109938

[Dan Beam, 2011-11-14 20:17:32]

On 2011/11/14 19:12:50, Evan Stade wrote:
> I don't think bind would work because at the time the return statement is
> executed, newTabView is still undefined.

Ah, yeah, OK.  First option it is, then!