229668 - Hang after typing keywords into firebird's address bar for "I'm feeling lucky" google search

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Mark Moorcroft]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7a) Gecko/20031229 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7a) Gecko/20031229 Firebird/0.7+

Entering a URL basename in the location bar like "apple" will hang Firebird.


Reproducible: Always

Steps to Reproduce:
1. Open Firebird
2. Type a partial url in the location bar
3.

Actual Results:  
Cursor converts to an arrow with spinner followed soon after by the
spinning pizza wheel.

Expected Results:  
Attempt completion of the URL (i.e. "www.apple.com")

Attempted several login accounts to confirm individual account settings not
the problem. Problem seems to have started around 12/23 nightly build. Nothing
in my firewall logs to indicate any issues there.

Comment 1

[Mark Moorcroft]

Verified.. I tried every build going back from today, and the 12/22 build is
where the problem vanishes. Camino has no such issues.

Comment 2

[Mark Moorcroft]

Bug still there in 10/31 MacOSX build

Comment 3

[Jerry Talkington]

This can be attributed to cvs <a
href="http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/docshell/base&command=DIFF_FRAMESET&root=/cvsroot&file=nsWebShell.cpp&rev1=1.622&rev2=1.623"
this checkin</a>.

I'll attach a gdb analysis and a patch in a bit.  

Comment 4

[Jerry Talkington]

^C when the program locks up.

(gdb) bt
#0  0x1471a33c in
nsCharSourceTraits<nsReadingIterator<char>>::readable_distance(nsReadingIterator<char>
const&, nsReadingIterator<char> const&) (first=@0xbfffdb10, last=@0xbfffdb30) at
../../dist/include/string/nsCharTraits.h:403
#1  0x146cf780 in nsACString::UncheckedAssignFromReadable(nsACString const&)
(this=0xbfffdde0, aReadable=@0xbfffdd10) at nsAString.cpp:795
#2  0x146cf560 in nsACString::do_AssignFromReadable(nsACString const&)
(this=0xbfffdde0, aReadable=@0xbfffdd10) at nsAString.cpp:764
#3  0x00d567c4 in nsACString::Assign(nsACString const&) (this=0xbfffdde0,
aReadable=@0xbfffdd10) at ../../dist/include/string/nsAString.h:512
#4  0x000c26a0 in net_ExtractURLScheme(nsACString const&, unsigned*, unsigned*,
nsACString*) (inURI=@0xbfffe0c0, startPos=0x0, endPos=0x0, scheme=0xbfffdde0) at
nsURLHelper.cpp:512
#5  0x000ca2c8 in nsIOService::ExtractScheme(nsACString const&, nsACString&)
(this=0x139ee750, inURI=@0xbfffe0c0, scheme=@0xbfffdde0) at nsIOService.cpp:365
#6  0x000ca438 in nsIOService::NewURI(nsACString const&, char const*, nsIURI*,
nsIURI**) (this=0x139ee750, aSpec=@0xbfffe0c0, aCharset=0x0, aBaseURI=0x0,
result=0xbfffdfa0) at nsIOService.cpp:385
#7  0x00d8207c in NS_NewURI(nsIURI**, nsACString const&, char const*, nsIURI*,
nsIIOService*) (result=0xbfffdfa0, spec=@0xbfffe0c0, charset=0x0, baseURI=0x0,
ioService=0x139ee750) at ../../../dist/include/necko/nsNetUtil.h:116
#8  0x00845e80 in nsWebShell::EndPageLoad(nsIWebProgress*, nsIChannel*,
unsigned) (this=0x1aad9810, aProgress=0x1aadc044, channel=0x1ac02ea0,
aStatus=2152398878) at nsWebShell.cpp:865
#9  0x008755cc in nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*,
unsigned, unsigned) (this=0x1aad9810, aProgress=0x1aadc044, aRequest=0x1ac02ea0,
aStateFlags=131088, aStatus=2152398878) at nsDocShell.cpp:4250
#10 0x0084d1a4 in nsDocLoaderImpl::FireOnStateChange(nsIWebProgress*,
nsIRequest*, int, unsigned) (this=0x1aadc030, aProgress=0x1aadc044,
aRequest=0x1ac02ea0, aStateFlags=131088, aStatus=2152398878) at nsDocLoader.cpp:1225
#11 0x0084c3a4 in nsDocLoaderImpl::doStopDocumentLoad(nsIRequest*, unsigned)
(this=0x1aadc030, request=0x1ac02ea0, aStatus=2152398878) at nsDocLoader.cpp:849
#12 0x0084c208 in nsDocLoaderImpl::DocLoaderIsEmpty() (this=0x1aadc030) at
nsDocLoader.cpp:760
#13 0x0084bfa0 in nsDocLoaderImpl::OnStopRequest(nsIRequest*, nsISupports*,
unsigned) (this=0x1aadc030, aRequest=0x1ac02ea0, aCtxt=0x0, aStatus=2152398878)
at nsDocLoader.cpp:689
#14 0x000cd334 in nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*,
unsigned) (this=0x1aadc200, request=0x1ac02ea0, ctxt=0x0, aStatus=2152398878) at
nsLoadGroup.cpp:695
#15 0x0013bed0 in nsHttpChannel::OnStopRequest(nsIRequest*, nsISupports*,
unsigned) (this=0x1ac02ea0, request=0x1aeece20, ctxt=0x0, status=2152398878) at
nsHttpChannel.cpp:3415
#16 0x00105e2c in nsInputStreamPump::OnStateStop() (this=0x1aeece20) at
nsInputStreamPump.cpp:498
#17 0x00105788 in nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*)
(this=0x1aeece20, stream=0x13940518) at nsInputStreamPump.cpp:339
#18 0x1472def8 in nsInputStreamReadyEvent::EventHandler(PLEvent*)
(plevent=0x1aeeceb4) at nsStreamUtils.cpp:116
#19 0x146a395c in PL_HandleEvent (self=0x1aeeceb4) at plevent.c:671
#20 0x146a37cc in PL_ProcessPendingEvents (self=0x139df5b0) at plevent.c:606
#21 0x146a3f68 in _md_EventReceiverProc (nextHandler=0xbfffe9b0,
inEvent=0x6f90422b, userData=0x139df5b0) at plevent.c:1565
#22 0x969a2c54 in DispatchEventToHandlers ()
#23 0x969a2fbc in SendEventToEventTargetInternal ()
#24 0x969a63d0 in SendEventToEventTargetWithOptions ()
#25 0x969b2940 in ToolboxEventDispatcherHandler(OpaqueEventHandlerCallRef*,
OpaqueEventRef*, void*) ()
#26 0x969a2d0c in DispatchEventToHandlers ()
#27 0x969a2fbc in SendEventToEventTargetInternal ()
#28 0x969b5494 in SendEventToEventTarget ()
#29 0x969b7258 in ToolboxEventDispatcher(OpaqueEventRef*) ()
#30 0x969c8740 in CallEventDispatchHook ()
#31 0x969b3c90 in TryEventDispatcher ()
#32 0x969a4570 in GetOrPeekEvent ()
#33 0x969a421c in GetNextEventMatchingMask ()
#34 0x969a8054 in WNEInternal ()
#35 0x969adf0c in WaitNextEvent ()
#36 0x00325688 in nsMacMessagePump::GetEvent(EventRecord&) (this=0x144e3d80,
theEvent=@0xbfffef90) at nsMacMessagePump.cpp:407
#37 0x003254ec in nsMacMessagePump::DoMessagePump() (this=0x144e3d80) at
nsMacMessagePump.cpp:304
#38 0x002fd0c0 in nsAppShell::Run() (this=0x144e3ae0) at nsAppShell.cpp:112
#39 0x00a0f510 in nsAppShellService::Run() (this=0x144e3a90) at
nsAppShellService.cpp:483
#40 0x00cd9dcc in main1(int, char**, nsISupports*, nsXREAppData const&) (argc=3,
argv=0xbffff43c, nativeApp=0x139576d0, aAppData=@0xbffff340) at nsAppRunner.cpp:1282
#41 0x00cda89c in xre_main(int, char**, nsXREAppData const&) (argc=3,
argv=0xbffff43c, aAppData=@0xbffff340) at nsAppRunner.cpp:1725
#42 0x00004dd0 in main (argc=3, argv=0xbffff43c) at nsBrowserApp.cpp:51

(gdb) n
nsWritingIterator<char>& copy_string<nsReadingIterator<char>,
nsWritingIterator<char> >(nsReadingIterator<char>&, nsReadingIterator<char>
const&, nsWritingIterator<char>&) (first=@0xbfffdb10, last=@0xbfffdb30,
result=@0xbfffdb50) at ../../../dist/include/string/nsAlgorithm.h:92
92              source_traits::advance(first, count_copied);
(gdb) 
88          while ( first != last )
(gdb) 
90              PRInt32 count_copied = PRInt32(sink_traits::write(result,
source_traits::read(first), source_traits::readable_distance(first, last)));
(gdb) 
92              source_traits::advance(first, count_copied);
(gdb) p count_copied
$1 = 0
(gdb) l
75            if ( *first == value )
76              ++result;
77          return result;
78        }
79      
80      template <class InputIterator, class OutputIterator>
81      inline
82      OutputIterator&
83      copy_string( InputIterator& first, const InputIterator& last,
OutputIterator& result )
84        {
85          typedef nsCharSourceTraits<InputIterator> source_traits;
86          typedef nsCharSinkTraits<OutputIterator>  sink_traits;
87      
88          while ( first != last )
89            {
90              PRInt32 count_copied = PRInt32(sink_traits::write(result,
source_traits::read(first), source_traits::readable_distance(first, last)));
91              NS_ASSERTION(count_copied > 0, "|copy_string| will never
terminate");
92              source_traits::advance(first, count_copied);
93            }
94      
95          return result;
96        }
97      
98      template <class InputIterator, class OutputIterator>
99      OutputIterator&
100     copy_string_backward( const InputIterator& first, InputIterator& last,
OutputIterator& result )
101       {
102         while ( first != last )
103           {
104             last.normalize_backward();
105             result.normalize_backward();
106             PRUint32 lengthToCopy = PRUint32( NS_MIN(last.size_backward(),
result.size_backward()) );
107             if ( first.fragment().mStart == last.fragment().mStart )
108               lengthToCopy = NS_MIN(lengthToCopy, PRUint32(last.get() -
first.get()));
109     

If we had debugging on, it would let us know we are in a while hole.

(gdb) f 8
#8  0x00845e80 in nsWebShell::EndPageLoad(nsIWebProgress*, nsIChannel*,
unsigned) (this=0x1aad9810, aProgress=0x1aadc044, channel=0x1ac02ea0,
aStatus=2152398878) at nsWebShell.cpp:865
865               NS_NewURI(getter_AddRefs(newURI),

Looking at Bonsai, we can see that this was changed between the 12/22 and 12/23
builds:
863             if(keywordsEnabled && (-1 == dotLoc)) {
864               // only send non-qualified hosts to the keyword server
865               NS_NewURI(getter_AddRefs(newURI),
866                         NS_LITERAL_CSTRING("keyword:") + host, nsnull);
867             } // end keywordsEnabled


#7  0x00d8207c in NS_NewURI(nsIURI**, nsACString const&, char const*, nsIURI*,
nsIIOService*) (result=0xbfffdfa0, spec=@0xbfffe0c0, charset=0x0, baseURI=0x0,
ioService=0x139ee750) at ../../../dist/include/necko/nsNetUtil.h:116
116             rv = ioService->NewURI(spec, charset, baseURI, result);
(gdb) p spec
$2 = (nsDependentCConcatenation &) @0xbfffe0c0: {
  <nsACString> = {
    _vptr$nsACString = 0x1475a7680x1472b410
<nsDependentCConcatenation::~nsDependentCConcatenation() at
../../dist/include/string/nsDependentConcatenation.h:227>
      
  }, 
  members of nsDependentCConcatenation: 
  mStrings =     {0xbfffe0d0,
    0xbfffe010}, 
  mFragmentIdentifierMask = 1
}

I think that this is only supposed to be one string...

(gdb) break 'NS_NewURI
NS_NewURI(nsIURI**, char const*, nsIURI*, nsIIOService*)
NS_NewURI(nsIURI**, nsACString const&, char const*, nsIURI*, nsIIOService*)
NS_NewURI(nsIURI**, nsAString const&, char const*, nsIURI*, nsIIOService*)


Hmm, looks like there may be a problem with joining the two strings.

(gdb) f 2
#2  0x146cf560 in nsACString::do_AssignFromReadable(nsACString const&)
(this=0xbfffdde0, aReadable=@0xbfffdd10) at nsAString.cpp:764
764           UncheckedAssignFromReadable(aReadable);
(gdb) down
#1  0x146cf780 in nsACString::UncheckedAssignFromReadable(nsACString const&)
(this=0xbfffdde0, aReadable=@0xbfffdd10) at nsAString.cpp:795
795             copy_string(aReadable.BeginReading(fromBegin),
aReadable.EndReading(fromEnd), BeginWriting(toBegin));

(gdb) p fromBegin
$6 = {
  mFragment = {
    mStart = 0xbfffe020 "apple", 
    mEnd = 0xbfffe025 "", 
    mFragmentIdentifier = 0x1
  }, 
  mPosition = 0xbfffe020 "apple", 
  mOwningString = 0xbfffdd10
}
(gdb) p fromEnd
$7 = {
  mFragment = {
    mStart = 0xd3d330 "keyword:", 
    mEnd = 0xd3d337 ":", 
    mFragmentIdentifier = 0x0
  }, 
  mPosition = 0xd3d337 ":", 
  mOwningString = 0xbfffdd10
}

I'm not sure exactly what's going on here.  I do know that it has something to
do with that cvs checkin, but I think that the problem is much deeper, perhaps
with the operator+ of nsACString (although I must admit that C is my
strongpoint, not C++.)

I'll try a tiny change that might fix this, but making the "keyword:word" one
single nsACString before passing it to NS_NewURI, by changing it to this:



         if(keywordsEnabled && (-1 == dotLoc)) {
           // only send non-qualified hosts to the keyword server
           nsCAutoString keywordSpec("keyword:");
           keywordSpec += host;

           NS_NewURI(getter_AddRefs(newURI),
                     keywordSpec, nsnull);
         } // end keywordsEnabled
       }
 

Bingo.  That works.  I'll attach a patch for it, but someone with a stronger
understanding of nsACStrings should take a look at why
NS_LITERAL_CSTRING("keyword:") + host
isn't working, if it even should.

Comment 5

[Jerry Talkington]

Attachment: Converts "keyword:blah" into one nsACString before passing it to NS_NewURI

Comment 6

[Jesse Ruderman]

Also happens on WinXP:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031223
Firebird/0.7+ (MozJF)

Comment 7

[Jesse Ruderman]

I see the following assertions repeated over and over:

###!!! ASSERTION: |copy_string| will never terminate: 'count_copied > 0', file
c:\buildmoz\mozilla\dist\include\string\nsAlgorithm.h, line 91
###!!! ASSERTION: You can't |write| into an |nsWritingIterator| with no space!:
'size_forward() > 0', file
c:\buildmoz\mozilla\dist\include\string\nsStringIterator.h, line 364

I don't think autocomplete dropdowns are involved, so -> General.

Comment 8

[Jesse Ruderman]

*** Bug 229893 has been marked as a duplicate of this bug. ***

Comment 9

[Jungshik Shin]

Jerry, you got it right. |'NS_LITERAL_STRING("blah") + other_string| cannot be
passed to a function. See the string guide at 
http://www.mozilla.org/projects/xpcom/string-guide.html#Concatenation

re : comment #7. I've seen that, too. When that assertion get triggered, there's
no way for the loop to end so that it enters an infinite loop. It might be a
good way to _force_ us to fix the root cause because the assertion shouldn't
fire in normal cases, but ....

Comment 10

[Jesse Ruderman]

[e-mail]: You nominated this bug to block 0.8, but I think this bug
is trunk-only.

Comment 11

[Johnny Stenback (:jst)]

I don't see anything in the string guide that says you can't pass concatenations
to a function, from what I can see it says the exact opposite, that you *can*
pass them to a function (as long as the concatenation is valid), and that's done
all over in Mozilla code. Now having said that, I know there are problems with
dealing with string concatenations, but only in some rare cases, darin knows more...

Comment 12

[Peter Van der Beken [:peterv]]

Comment on attachment 138229 [details] [diff] [review]
Converts "keyword:blah" into one nsACString before passing it to NS_NewURI

That seems to be the only case of |NS_LITERAL_STRING(...) + foo| I introduced
in that patch (I removed a bunch of those too).

Comment 13

[Peter Van der Beken [:peterv]]

Bug 136980 is the string bug.

Comment 14

[Jungshik Shin]

Ooops. Sorry I completely misread it. I don't know how I could....Thanks for the 
pointer to bug 136980. I recently stumbled upon exactly the same problem. 

Comment 15

[Jungshik Shin]

Comment on attachment 138229 [details] [diff] [review]
Converts "keyword:blah" into one nsACString before passing it to NS_NewURI

r=jshin

Comment 16

[Peter Van der Beken [:peterv]]

Fix checked in. Thanks for the patch and review.

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

could you mention the bug# of the string bug in the comment, so this instance
can be fixed once the string bug is fixed?

Comment 18

[Olivier Cahagne]

*** Bug 229325 has been marked as a duplicate of this bug. ***

Comment 19

[Jason Barnabe (np)]

*** Bug 229950 has been marked as a duplicate of this bug. ***

Comment 20

[benc]

-> Browser: DocShell

if you are going to patch the trunk, please move the bug to the right component.

Can someone verify the fix and change state of this bug?
Assign your self QA if you do...