Speculative resource prefetching - Test fixes.

Speculative resource prefetching - Test fixes.

- Removing dcheck that two navigations on the same renderview with the same url should not have the same creation time. It looks like it is possible for tests to be able to do this and theoretically it is possible for this to happen in production. The code already this handles this case well, so the check will not be missed.

- Add a check to see if WebContents returns empty URL before calling OnNavigationComplete.


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

[Shishir, 2012-07-31 22:52:08]

Please take a look.

[dominich, 2012-07-31 23:08:50]

https://chromiumcodereview.appspot.com/10825118/diff/1/chrome/browser/predict...
File chrome/browser/predictors/resource_prefetch_predictor.cc (right):

https://chromiumcodereview.appspot.com/10825118/diff/1/chrome/browser/predict...
chrome/browser/predictors/resource_prefetch_predictor.cc:411: // WebContents can
return an empty URL if the is no navigation entry yet.
nit: the -> there

https://chromiumcodereview.appspot.com/10825118/diff/1/chrome/browser/predict...
chrome/browser/predictors/resource_prefetch_predictor.cc:412: if
(!navigation_id.main_frame_url.is_empty())
Also, expand the comment to explain 1) when this might be the case and 2) if
this is a problem in terms of not seeing navigations we might be interested in.

https://chromiumcodereview.appspot.com/10825118/diff/1/chrome/browser/predict...
chrome/browser/predictors/resource_prefetch_predictor.cc:413:
OnNavigationComplete(navigation_id);
When do we erase this from inflight navigations if we don't call
OnNavigationComplete?

[Shishir, 2012-07-31 23:29:35]

https://chromiumcodereview.appspot.com/10825118/diff/1/chrome/browser/predict...
File chrome/browser/predictors/resource_prefetch_predictor.cc (right):

https://chromiumcodereview.appspot.com/10825118/diff/1/chrome/browser/predict...
chrome/browser/predictors/resource_prefetch_predictor.cc:411: // WebContents can
return an empty URL if the is no navigation entry yet.
On 2012/07/31 23:08:50, dominich wrote:
> nit: the -> there

Done.

https://chromiumcodereview.appspot.com/10825118/diff/1/chrome/browser/predict...
chrome/browser/predictors/resource_prefetch_predictor.cc:412: if
(!navigation_id.main_frame_url.is_empty())
On 2012/07/31 23:08:50, dominich wrote:
> Also, expand the comment to explain 1) when this might be the case and 2) if
> this is a problem in terms of not seeing navigations we might be interested
in.

Its the way the GetUrl() function call in webcontents is designed. It uses
NavigationController.GetActiveEntry which can be NULL. I have only seen this in
a test so far, but it may occur in production chrome. Appended the same comment
above.

https://chromiumcodereview.appspot.com/10825118/diff/1/chrome/browser/predict...
chrome/browser/predictors/resource_prefetch_predictor.cc:413:
OnNavigationComplete(navigation_id);
On 2012/07/31 23:08:50, dominich wrote:
> When do we erase this from inflight navigations if we don't call
> OnNavigationComplete?

In CleanupOldNavigations.

[dominich, 2012-08-01 00:40:24]

lgtm

[commit-bot: I haz the power, 2012-08-01 00:41:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/shishir@chromium.org/10825118/9001

[commit-bot: I haz the power, 2012-08-01 02:07:19]

Change committed as 149353