659177 - nsAboutProtocolHandler::StartClone needs to change signature to continue overriding inherited method

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Daniel Holbert [:dholbert]]

Bug 308590 changed nsSimpleURI::StartClone and its subclass's version nsSimpleNestedURI::StartClone to take one argument.

I missed that nsSimpleNestedURI *itself* had a subclass with a version that needed changing, too.  As a result, we currently get this build warning:
{
  nsSimpleNestedURI.h:80:26: warning: ‘virtual nsSimpleURI* nsSimpleNestedURI::StartClone(nsSimpleURI::RefHandlingEnum)’ was hidden
  nsAboutProtocolHandler.h:97:26: warning:   by ‘virtual nsSimpleURI* nsNestedAboutURI::StartClone()’
}

nsNestedAboutURI::StartClone needs to change to take an argument in order to continue overriding properly.

(I verified that this is the only instance of this type of bug by looking for "::Clone" and "::StartClone" in a build log)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

Ultimately, it'd be nice for nsSimpleNestedURI::StartClone to replace its "new nsSimpleNestedURI()" call with some virtual function that nsNestedAboutURI can override (returning a "new nsNestedAboutURI" instead).

Then, we'll be able to get rid of nsNestedAboutURI's StartClone specialization.

However, for now, this is a simpler change.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 534605 [details] [diff] [review]
fix

r=me

Comment 3

[Boris Zbarsky [:bzbarsky]]

This needs to land in Fx6.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: hg changeset for checkin

Comment 5

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/bb5904c365a2

Comment 6

[Johnathan Nightingale [:johnath]]

I agree this needs to land, but doesn't really need to live on the tracking radar - a=me for aurora landing.

Comment 7

[Daniel Holbert [:dholbert]]

Comment on attachment 534605 [details] [diff] [review]
fix

Requesting approval to land on aurora, per comment 3.

Pretty trivial, low-risk fix - as described in comment 0, this just adds an argument to a method on nsNestedAboutURI, mirroring bug 308590's changes to the same method in nsNestedAboutURI's parent class.  This is necessary for the method to override correctly.

Comment 8

[Daniel Holbert [:dholbert]]

Comment on attachment 534605 [details] [diff] [review]
fix

oops, johnath beat me to the punch, a+'ing the newer revision of the patch. :) Canceling duplicate approval request.

Comment 9

[Daniel Holbert [:dholbert]]

Comment on attachment 534688 [details] [diff] [review]
hg changeset for checkin

gah, sorry, this is already in Aurora -- it landed on m-c a few hours before the branch point.

Canceling approval+, for clarity.