Instrument SdchOwner for clock nullification.

Instrument SdchOwner for clock nullification.

BUG=454198
R=ellyjones@chromium.org

Committed: https://crrev.com/f5047a459f754c9b64004b00fe00adfdd8d0b37a
Cr-Commit-Position: refs/heads/master@{#317364}

[Randy Smith (Not in Mondays), 2015-02-18 21:18:53]

Elly: PTAL?  I'd specifically like you to check my use of
might_have_observers()--I believe the only case in which that'll return true
when there *aren't* observers is if a RemoveObservers happens during a
observation notification, which shouldn't happen in this code, but I'd like
another pair of eyes to confirm that.

[Randy Smith (Not in Mondays), 2015-02-19 19:19:32]

rdsmith@chromium.org changed reviewers:
+ mmenke@chromium.org

[Randy Smith (Not in Mondays), 2015-02-19 19:19:33]

Matt: Do you have time for a quick review?  I think Elly's busy, and this is
small and I'd rather get it in sooner than later.

[mmenke, 2015-02-19 19:20:19]

I had an interview today and a number of other reviews...Still trying to
get to it.

On Thu, Feb 19, 2015 at 2:19 PM, <rdsmith@chromium.org> wrote:

> Matt: Do you have time for a quick review?  I think Elly's busy, and this
> is
> small and I'd rather get it in sooner than later.
>
>
> https://codereview.chromium.org/935073003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Randy Smith (Not in Mondays), 2015-02-19 19:23:30]

On 2015/02/19 19:20:19, mmenke wrote:
> I had an interview today and a number of other reviews...Still trying to
> get to it.
> 
> On Thu, Feb 19, 2015 at 2:19 PM, <mailto:rdsmith@chromium.org> wrote:
> 
> > Matt: Do you have time for a quick review?  I think Elly's busy, and this
> > is
> > small and I'd rather get it in sooner than later.
> >
> >
> > https://codereview.chromium.org/935073003/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Cool; thanks for the expectation setting.  I'll take the review of the person
who gets to it first.

[mmenke, 2015-02-19 19:59:12]

No problem!  Hrm...1,200 lines of code is small?  :)

On Thu, Feb 19, 2015 at 2:23 PM, <rdsmith@chromium.org> wrote:

> On 2015/02/19 19:20:19, mmenke wrote:
>
>> I had an interview today and a number of other reviews...Still trying to
>> get to it.
>>
>
>  On Thu, Feb 19, 2015 at 2:19 PM, <mailto:rdsmith@chromium.org> wrote:
>>
>
>  > Matt: Do you have time for a quick review?  I think Elly's busy, and
>> this
>> > is
>> > small and I'd rather get it in sooner than later.
>> >
>> >
>> > https://codereview.chromium.org/935073003/
>> >
>>
>
>  To unsubscribe from this group and stop receiving emails from it, send an
>>
> email
>
>> to mailto:chromium-reviews+unsubscribe@chromium.org.
>>
>
> Cool; thanks for the expectation setting.  I'll take the review of the
> person
> who gets to it first.
>
>
> https://codereview.chromium.org/935073003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Randy Smith (Not in Mondays), 2015-02-19 20:23:01]

On 2015/02/19 19:59:12, mmenke wrote:
> No problem!  Hrm...1,200 lines of code is small?  :)

Sorry, I count 53 and there's a lot of code duplication?  (Unless you're
referring to your current standards in code reviews, in which case I'll just
quake in terror over in the corner :-}.)

> 
> On Thu, Feb 19, 2015 at 2:23 PM, <mailto:rdsmith@chromium.org> wrote:
> 
> > On 2015/02/19 19:20:19, mmenke wrote:
> >
> >> I had an interview today and a number of other reviews...Still trying to
> >> get to it.
> >>
> >
> >  On Thu, Feb 19, 2015 at 2:19 PM, <mailto:rdsmith@chromium.org> wrote:
> >>
> >
> >  > Matt: Do you have time for a quick review?  I think Elly's busy, and
> >> this
> >> > is
> >> > small and I'd rather get it in sooner than later.
> >> >
> >> >
> >> > https://codereview.chromium.org/935073003/
> >> >
> >>
> >
> >  To unsubscribe from this group and stop receiving emails from it, send an
> >>
> > email
> >
> >> to mailto:chromium-reviews+unsubscribe@chromium.org.
> >>
> >
> > Cool; thanks for the expectation setting.  I'll take the review of the
> > person
> > who gets to it first.
> >
> >
> > https://codereview.chromium.org/935073003/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[mmenke, 2015-02-19 20:26:26]

On 2015/02/19 20:23:01, rdsmith wrote:
> On 2015/02/19 19:59:12, mmenke wrote:
> > No problem!  Hrm...1,200 lines of code is small?  :)
> 
> Sorry, I count 53 and there's a lot of code duplication?  (Unless you're
> referring to your current standards in code reviews, in which case I'll just
> quake in terror over in the corner :-}.)
> 
> > 
> > On Thu, Feb 19, 2015 at 2:23 PM, <mailto:rdsmith@chromium.org> wrote:
> > 
> > > On 2015/02/19 19:20:19, mmenke wrote:
> > >
> > >> I had an interview today and a number of other reviews...Still trying to
> > >> get to it.
> > >>
> > >
> > >  On Thu, Feb 19, 2015 at 2:19 PM, <mailto:rdsmith@chromium.org> wrote:
> > >>
> > >
> > >  > Matt: Do you have time for a quick review?  I think Elly's busy, and
> > >> this
> > >> > is
> > >> > small and I'd rather get it in sooner than later.
> > >> >
> > >> >
> > >> > https://codereview.chromium.org/935073003/
> > >> >
> > >>
> > >
> > >  To unsubscribe from this group and stop receiving emails from it, send an
> > >>
> > > email
> > >
> > >> to mailto:chromium-reviews+unsubscribe@chromium.org.
> > >>
> > >
> > > Cool; thanks for the expectation setting.  I'll take the review of the
> > > person
> > > who gets to it first.
> > >
> > >
> > > https://codereview.chromium.org/935073003/
> > >
> > 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.

Oh...I completely didn't notice this was another CL.  I just assumed this was
the one you passed to Elly...And was a bit curious about the race to get it in.

[mmenke, 2015-02-19 20:42:57]

If this isn't too illuminating, could also try pushing the URL onto the stack.

https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc
File net/sdch/sdch_owner.cc (right):

https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
net/sdch/sdch_owner.cc:109: CHECK(clock_.get());
clock_.reset(), to explicitly NULL it out (Admittedly, that + deadbeef may be a
bit redundant, but you're checking the clock everywhere, not deadbeef)

https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
net/sdch/sdch_owner.cc:159: CHECK(clock_.get());
Should you also CHECK_EQ(0, destroyed_) first, in all the places you do this?

[mmenke, 2015-02-19 20:48:42]

https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc
File net/sdch/sdch_owner.cc (right):

https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
net/sdch/sdch_owner.cc:159: CHECK(clock_.get());
On 2015/02/19 20:42:57, mmenke wrote:
> Should you also CHECK_EQ(0, destroyed_) first, in all the places you do this?

Actually, CHECK_NE(destroyed_, 0xdeadbeef);

[Randy Smith (Not in Mondays), 2015-02-19 20:55:32]

I don't at the moment see what I'd get from looking at the URL; I can't imagine
any way this could be URL related.  (I acknowledge that I can't currently
imagine any way it could be happening, but I'd still like to restrain my
debugging to areas where I have some sense that the info could be useful.)

PTAL?

https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc
File net/sdch/sdch_owner.cc (right):

https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
net/sdch/sdch_owner.cc:109: CHECK(clock_.get());
On 2015/02/19 20:42:57, mmenke wrote:
> clock_.reset(), to explicitly NULL it out (Admittedly, that + deadbeef may be
a
> bit redundant, but you're checking the clock everywhere, not deadbeef)

Done, though that may mean I substantially spike the crash frequency.  OTOH,
that's probably ok :-}.

https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
net/sdch/sdch_owner.cc:159: CHECK(clock_.get());
On 2015/02/19 20:42:57, mmenke wrote:
> Should you also CHECK_EQ(0, destroyed_) first, in all the places you do this?

Done.

https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
net/sdch/sdch_owner.cc:159: CHECK(clock_.get());
On 2015/02/19 20:48:41, mmenke wrote:
> On 2015/02/19 20:42:57, mmenke wrote:
> > Should you also CHECK_EQ(0, destroyed_) first, in all the places you do
this?
> 
> Actually, CHECK_NE(destroyed_, 0xdeadbeef);

Disagreed, actually.  If it's anything other than zero, that's either
destruction (possibly + later usage) or a memory sprayer, and in either case, I
think I want to trip on it.  Which maybe means I shouldn't use 0xdeadbeef, but
if I ever do manage to get a minidump under a debugger for this, I'll be curious
if the pattern still looks like that.  It's less likely something else would set
the memory to 0xdeadbeef than to, say, 1.

[mmenke, 2015-02-19 20:59:27]

On 2015/02/19 20:55:32, rdsmith wrote:
> I don't at the moment see what I'd get from looking at the URL; I can't
imagine
> any way this could be URL related.  (I acknowledge that I can't currently
> imagine any way it could be happening, but I'd still like to restrain my
> debugging to areas where I have some sense that the info could be useful.)

I strongly suspect we're either running into issues a URLRequestContext other
than the main request context, or some internal Chrome module that's incorrectly
managing the lifetimes of its own URLRequest, rather than requests made from the
web.  If we knew it was (for example) a google sync URL, we would know it's an
issue with the URLRequestContext used for sync.

> PTAL?
> 
> https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc
> File net/sdch/sdch_owner.cc (right):
> 
>
https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
> net/sdch/sdch_owner.cc:109: CHECK(clock_.get());
> On 2015/02/19 20:42:57, mmenke wrote:
> > clock_.reset(), to explicitly NULL it out (Admittedly, that + deadbeef may
be
> a
> > bit redundant, but you're checking the clock everywhere, not deadbeef)
> 
> Done, though that may mean I substantially spike the crash frequency.  OTOH,
> that's probably ok :-}.
> 
>
https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
> net/sdch/sdch_owner.cc:159: CHECK(clock_.get());
> On 2015/02/19 20:42:57, mmenke wrote:
> > Should you also CHECK_EQ(0, destroyed_) first, in all the places you do
this?
> 
> Done.
> 
>
https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
> net/sdch/sdch_owner.cc:159: CHECK(clock_.get());
> On 2015/02/19 20:48:41, mmenke wrote:
> > On 2015/02/19 20:42:57, mmenke wrote:
> > > Should you also CHECK_EQ(0, destroyed_) first, in all the places you do
> this?
> > 
> > Actually, CHECK_NE(destroyed_, 0xdeadbeef);
> 
> Disagreed, actually.  If it's anything other than zero, that's either
> destruction (possibly + later usage) or a memory sprayer, and in either case,
I
> think I want to trip on it.  Which maybe means I shouldn't use 0xdeadbeef, but
> if I ever do manage to get a minidump under a debugger for this, I'll be
curious
> if the pattern still looks like that.  It's less likely something else would
set
> the memory to 0xdeadbeef than to, say, 1.

Ah, right...I was thinking along the right lines the first time.

[mmenke, 2015-02-19 21:00:38]

LGTM

[Randy Smith (Not in Mondays), 2015-02-20 00:45:09]

New patchsets have been uploaded after l-g-t-m from mmenke@chromium.org

[Randy Smith (Not in Mondays), 2015-02-20 00:46:54]

On 2015/02/19 20:59:27, mmenke wrote:
> On 2015/02/19 20:55:32, rdsmith wrote:
> > I don't at the moment see what I'd get from looking at the URL; I can't
> imagine
> > any way this could be URL related.  (I acknowledge that I can't currently
> > imagine any way it could be happening, but I'd still like to restrain my
> > debugging to areas where I have some sense that the info could be useful.)
> 
> I strongly suspect we're either running into issues a URLRequestContext other
> than the main request context, or some internal Chrome module that's
incorrectly
> managing the lifetimes of its own URLRequest, rather than requests made from
the
> web.  If we knew it was (for example) a google sync URL, we would know it's an
> issue with the URLRequestContext used for sync.

Gotcha, that makes sense.  I've rewritten to include the URL, but it's enough of
a change that I'm afraid I need a re-review.  PTAL?

> 
> > PTAL?
> > 
> > https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc
> > File net/sdch/sdch_owner.cc (right):
> > 
> >
>
https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
> > net/sdch/sdch_owner.cc:109: CHECK(clock_.get());
> > On 2015/02/19 20:42:57, mmenke wrote:
> > > clock_.reset(), to explicitly NULL it out (Admittedly, that + deadbeef may
> be
> > a
> > > bit redundant, but you're checking the clock everywhere, not deadbeef)
> > 
> > Done, though that may mean I substantially spike the crash frequency.  OTOH,
> > that's probably ok :-}.
> > 
> >
>
https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
> > net/sdch/sdch_owner.cc:159: CHECK(clock_.get());
> > On 2015/02/19 20:42:57, mmenke wrote:
> > > Should you also CHECK_EQ(0, destroyed_) first, in all the places you do
> this?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/935073003/diff/1/net/sdch/sdch_owner.cc#newco...
> > net/sdch/sdch_owner.cc:159: CHECK(clock_.get());
> > On 2015/02/19 20:48:41, mmenke wrote:
> > > On 2015/02/19 20:42:57, mmenke wrote:
> > > > Should you also CHECK_EQ(0, destroyed_) first, in all the places you do
> > this?
> > > 
> > > Actually, CHECK_NE(destroyed_, 0xdeadbeef);
> > 
> > Disagreed, actually.  If it's anything other than zero, that's either
> > destruction (possibly + later usage) or a memory sprayer, and in either
case,
> I
> > think I want to trip on it.  Which maybe means I shouldn't use 0xdeadbeef,
but
> > if I ever do manage to get a minidump under a debugger for this, I'll be
> curious
> > if the pattern still looks like that.  It's less likely something else would
> set
> > the memory to 0xdeadbeef than to, say, 1.
> 
> Ah, right...I was thinking along the right lines the first time.

[mmenke, 2015-02-20 15:46:07]

LGTM

[Randy Smith (Not in Mondays), 2015-02-20 15:54:37]

The CQ bit was checked by rdsmith@chromium.org

[commit-bot: I haz the power, 2015-02-20 15:55:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/935073003/60001

[commit-bot: I haz the power, 2015-02-20 16:05:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-20 16:05:35]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[mmenke, 2015-02-20 16:31:32]

On 2015/02/20 16:05:35, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   ios_dbg_simulator on tryserver.chromium.mac (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

Oops...I should have caught that.  :(

[Randy Smith (Not in Mondays), 2015-02-20 17:24:18]

New patchsets have been uploaded after l-g-t-m from mmenke@chromium.org

[Randy Smith (Not in Mondays), 2015-02-20 17:26:07]

On 2015/02/20 16:31:32, mmenke wrote:
> On 2015/02/20 16:05:35, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   ios_dbg_simulator on tryserver.chromium.mac (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
> 
> Oops...I should have caught that.  :(

You and me both :-{.  I compiled this time both with and without chromeos=1
(just sdch_owner.cc) before uploading.  You're welcome to look now, or I'll ping
you after try jobs complete successfully; up to you.

[mmenke, 2015-02-20 17:30:09]

https://codereview.chromium.org/935073003/diff/80001/net/sdch/sdch_owner.cc
File net/sdch/sdch_owner.cc (right):

https://codereview.chromium.org/935073003/diff/80001/net/sdch/sdch_owner.cc#n...
net/sdch/sdch_owner.cc:155: AssertNotDestroyedAndClockNotNull(__LINE__,
dictionary_url);
Sorry, I really wasn't thinking when I signed off...This is actually the wrong
URL.  I blame it on today being Friday (It's all Friday's fault, dangit!)  The
URL we'd actually want is the URL of the URLRequest that's calling into the
SdchManager to request the URL, which is a couple layers up in the stack.

[Randy Smith (Not in Mondays), 2015-02-20 17:34:45]

On 2015/02/20 17:30:09, mmenke wrote:
> https://codereview.chromium.org/935073003/diff/80001/net/sdch/sdch_owner.cc
> File net/sdch/sdch_owner.cc (right):
> 
>
https://codereview.chromium.org/935073003/diff/80001/net/sdch/sdch_owner.cc#n...
> net/sdch/sdch_owner.cc:155: AssertNotDestroyedAndClockNotNull(__LINE__,
> dictionary_url);
> Sorry, I really wasn't thinking when I signed off...This is actually the wrong
> URL.  I blame it on today being Friday (It's all Friday's fault, dangit!)  The
> URL we'd actually want is the URL of the URLRequest that's calling into the
> SdchManager to request the URL, which is a couple layers up in the stack.

Right, right.  Arggh.  This is what happens when I try to do something (i.e.
incorporate a valid comment) too quickly.

I'm going to rip out the URL tracking (which I think means reverting to PS3),
land this (I think you LGTMd PS3), and then contemplate whether the amount of
plumbing necessary to get the referrer URL is worth it.

[Randy Smith (Not in Mondays), 2015-02-20 17:37:24]

The CQ bit was checked by rdsmith@chromium.org

[mmenke, 2015-02-20 17:37:43]

On 2015/02/20 17:34:45, rdsmith wrote:
> On 2015/02/20 17:30:09, mmenke wrote:
> > https://codereview.chromium.org/935073003/diff/80001/net/sdch/sdch_owner.cc
> > File net/sdch/sdch_owner.cc (right):
> > 
> >
>
https://codereview.chromium.org/935073003/diff/80001/net/sdch/sdch_owner.cc#n...
> > net/sdch/sdch_owner.cc:155: AssertNotDestroyedAndClockNotNull(__LINE__,
> > dictionary_url);
> > Sorry, I really wasn't thinking when I signed off...This is actually the
wrong
> > URL.  I blame it on today being Friday (It's all Friday's fault, dangit!) 
The
> > URL we'd actually want is the URL of the URLRequest that's calling into the
> > SdchManager to request the URL, which is a couple layers up in the stack.
> 
> Right, right.  Arggh.  This is what happens when I try to do something (i.e.
> incorporate a valid comment) too quickly.

And when I rush to sign off too quickly.

> I'm going to rip out the URL tracking (which I think means reverting to PS3),
> land this (I think you LGTMd PS3), and then contemplate whether the amount of
> plumbing necessary to get the referrer URL is worth it.

I'm fine with landing PS3.

[commit-bot: I haz the power, 2015-02-20 17:38:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/935073003/100001

[Randy Smith (Not in Mondays), 2015-02-20 17:40:19]

On 2015/02/20 17:37:43, mmenke wrote:
> On 2015/02/20 17:34:45, rdsmith wrote:
> > On 2015/02/20 17:30:09, mmenke wrote:
> > >
https://codereview.chromium.org/935073003/diff/80001/net/sdch/sdch_owner.cc
> > > File net/sdch/sdch_owner.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/935073003/diff/80001/net/sdch/sdch_owner.cc#n...
> > > net/sdch/sdch_owner.cc:155: AssertNotDestroyedAndClockNotNull(__LINE__,
> > > dictionary_url);
> > > Sorry, I really wasn't thinking when I signed off...This is actually the
> wrong
> > > URL.  I blame it on today being Friday (It's all Friday's fault, dangit!) 
> The
> > > URL we'd actually want is the URL of the URLRequest that's calling into
the
> > > SdchManager to request the URL, which is a couple layers up in the stack.
> > 
> > Right, right.  Arggh.  This is what happens when I try to do something (i.e.
> > incorporate a valid comment) too quickly.
> 
> And when I rush to sign off too quickly.
> 
> > I'm going to rip out the URL tracking (which I think means reverting to
PS3),
> > land this (I think you LGTMd PS3), and then contemplate whether the amount
of
> > plumbing necessary to get the referrer URL is worth it.
> 
> I'm fine with landing PS3.

Looking at the plumbing, it'll be trivial to wrap the request_url in the
callback once the dictionary persistence CL lands, and I expect some version of
that to land soon (even if it's behind a finch flag, the new callback mechanism
for SdchDictionaryFetcher will be in place).  So I'll wait on that CL and add
the URL tracking when it lands.

[mmenke, 2015-02-20 17:43:11]

On 2015/02/20 17:40:19, rdsmith wrote:
> On 2015/02/20 17:37:43, mmenke wrote:
> > On 2015/02/20 17:34:45, rdsmith wrote:
> > > On 2015/02/20 17:30:09, mmenke wrote:
> > > >
> https://codereview.chromium.org/935073003/diff/80001/net/sdch/sdch_owner.cc
> > > > File net/sdch/sdch_owner.cc (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/935073003/diff/80001/net/sdch/sdch_owner.cc#n...
> > > > net/sdch/sdch_owner.cc:155: AssertNotDestroyedAndClockNotNull(__LINE__,
> > > > dictionary_url);
> > > > Sorry, I really wasn't thinking when I signed off...This is actually the
> > wrong
> > > > URL.  I blame it on today being Friday (It's all Friday's fault,
dangit!) 
> > The
> > > > URL we'd actually want is the URL of the URLRequest that's calling into
> the
> > > > SdchManager to request the URL, which is a couple layers up in the
stack.
> > > 
> > > Right, right.  Arggh.  This is what happens when I try to do something
(i.e.
> > > incorporate a valid comment) too quickly.
> > 
> > And when I rush to sign off too quickly.
> > 
> > > I'm going to rip out the URL tracking (which I think means reverting to
> PS3),
> > > land this (I think you LGTMd PS3), and then contemplate whether the amount
> of
> > > plumbing necessary to get the referrer URL is worth it.
> > 
> > I'm fine with landing PS3.
> 
> Looking at the plumbing, it'll be trivial to wrap the request_url in the
> callback once the dictionary persistence CL lands, and I expect some version
of
> that to land soon (even if it's behind a finch flag, the new callback
mechanism
> for SdchDictionaryFetcher will be in place).  So I'll wait on that CL and add
> the URL tracking when it lands.

I was thinking the place to do it would just be to always stick it on the stack
when the URLRequest calls into to URLRequestManager to request a dictionary.

[commit-bot: I haz the power, 2015-02-20 19:20:55]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-02-20 19:21:26]

Patchset 6 (id:??) landed as
https://crrev.com/f5047a459f754c9b64004b00fe00adfdd8d0b37a
Cr-Commit-Position: refs/heads/master@{#317364}

[mmenke, 2015-05-22 14:49:26]

On 2015/02/20 19:21:26, I haz the power (commit-bot) wrote:
> Patchset 6 (id:??) landed as
> https://crrev.com/f5047a459f754c9b64004b00fe00adfdd8d0b37a
> Cr-Commit-Position: refs/heads/master@{#317364}

Hey randy, elly:  This can be reverted now, as can
https://codereview.chromium.org/998803003, right?

[Randy Smith (Not in Mondays), 2015-05-23 00:03:19]

On 2015/05/22 14:49:26, mmenke wrote:
> On 2015/02/20 19:21:26, I haz the power (commit-bot) wrote:
> > Patchset 6 (id:??) landed as
> > https://crrev.com/f5047a459f754c9b64004b00fe00adfdd8d0b37a
> > Cr-Commit-Position: refs/heads/master@{#317364}
> 
> Hey randy, elly:  This can be reverted now, as can
> https://codereview.chromium.org/998803003, right?

Arggh, yes, I was going to do that when http://crbug.com/447208 was retired, but
that took a long time and It slipped my mind.  I'll take care of it.