Fix a flicker of the URL bar after you enter it. Since no tab contents was...

Fix a flicker of the URL bar after you enter it. The code
inBrowser::OpenURLFromTab was not using the current tab when no source was
specified. This means we didn't update the correct tab.

This change uses the computed current URL for updating, and also clarifies the
comments in the TabContentsDelegate about what NULL means.

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

[brettw, 2009-04-08 17:41:15]

I removed the assert. This function is used by about one brazillion unit tests,
and it's not always convienient to get the current tab.

[Peter Kasting, 2009-04-08 19:34:42]

http://codereview.chromium.org/63125/diff/1003/1005
File chrome/browser/browser.cc (right):

http://codereview.chromium.org/63125/diff/1003/1005#newcode639
Line 639: OpenURLFromTab(GetSelectedTabContents(),
Did you also audit other callers of OpenURL() to see if anyone else should do
this?

http://codereview.chromium.org/63125/diff/1003/1004
File chrome/browser/tab_contents/tab_contents_delegate.h (right):

http://codereview.chromium.org/63125/diff/1003/1004#newcode10
Line 10: #include "base/logging.h"
Don't need this if you're not DCHECKing.  But...

http://codereview.chromium.org/63125/diff/1003/1004#newcode30
Line 30: // In non-test code, do not use the disposition CURRENT_TAB since the
UI
There must be some way to compile- or run-time test this.  Either we can DCHECK
here and then make an #if UNIT_TEST function UnitTestOpenURL() that doesn't
check, and switch tests that need to do this to that, or have a run-time check
for a unit test, or bite the bullet and satisfy this constraint in every unit
test, or something.  But this is too subtle to not have an actual check for.

Another possibility is to remove OpenURL() and force callers to decide that they
want to pass NULL for the "source", and then comment what the effect of that
would be.

[brettw, 2009-04-08 20:41:51]

I figured out the actual problem this time.

After studying calls to OpenURL, I realized a NULL source is supposed to
indicate the "current tab," and the bug was that I wasn't using the computed
current tab properly.

[Peter Kasting, 2009-04-08 20:44:47]

LGTM on the code change.  Is there any way to add a UI regression test for this?
 I don't know one offhand but maybe you're smarter than me.

http://codereview.chromium.org/63125/diff/14/1011
File chrome/browser/tab_contents/tab_contents_delegate.h (right):

http://codereview.chromium.org/63125/diff/14/1011#newcode32
Line 32: void OpenURL(const GURL& url, const GURL& referrer,
Was it safe to un-virtualize this?  No implementers actually overrode it?