Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(229)

Issue 5290005: Use the correct signal to detect failed navigations (Closed)

Created:
10 years ago by jochen (gone - plz use gerrit)
Modified:
9 years, 7 months ago
Reviewers:
yzshen, Matt Perry
CC:
chromium-reviews, ben+cc_chromium.org, Erik does not do reviews, Paweł Hajdan Jr., Aaron Boodman, pam+watch_chromium.org, brettw-cc_chromium.org
Visibility:
Public.

Description

Use the correct signal to detect failed navigations BUG=50943 TEST=unit_tests:FrameNavigationStateTest.*, browser_tests:ExtensionApiTest.WebNavigation* Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=67999

Patch Set 1 #

Patch Set 2 : rebase #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -19 lines) Patch
M chrome/browser/extensions/extension_webnavigation_api.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/extensions/extension_webnavigation_api.cc View 1 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/browser/extensions/extension_webnavigation_unittest.cc View 1 5 chunks +11 lines, -14 lines 0 comments Download
M chrome/browser/tab_contents/provisional_load_details.h View 1 3 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/tab_contents/provisional_load_details.cc View 1 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/browser/tab_contents/tab_contents.cc View 1 3 chunks +4 lines, -3 lines 2 comments Download

Messages

Total messages: 5 (0 generated)
jochen (gone - plz use gerrit)
please review
10 years ago (2010-12-01 10:25:05 UTC) #1
yzshen
http://codereview.chromium.org/5290005/diff/2001/chrome/browser/tab_contents/tab_contents.cc File chrome/browser/tab_contents/tab_contents.cc (right): http://codereview.chromium.org/5290005/diff/2001/chrome/browser/tab_contents/tab_contents.cc#newcode2256 chrome/browser/tab_contents/tab_contents.cc:2256: url, std::string(), false, false, frame_id); I am not sure ...
10 years ago (2010-12-01 18:21:50 UTC) #2
Matt Perry
LGTM, though I'm confused. http://codereview.chromium.org/5290005/diff/2001/chrome/browser/tab_contents/tab_contents.cc File chrome/browser/tab_contents/tab_contents.cc (right): http://codereview.chromium.org/5290005/diff/2001/chrome/browser/tab_contents/tab_contents.cc#newcode2256 chrome/browser/tab_contents/tab_contents.cc:2256: url, std::string(), false, false, frame_id); ...
10 years ago (2010-12-01 20:51:09 UTC) #3
jochen (gone - plz use gerrit)
In short: yes, that's the only event. When a navigation fails (no matter at which ...
10 years ago (2010-12-02 12:59:10 UTC) #4
yzshen
10 years ago (2010-12-02 17:42:39 UTC) #5
LGTM

On 2010/12/02 12:59:10, jochen wrote:
> In short: yes, that's the only event.
It might be good to add a comment for this.
 
> When a navigation fails (no matter at which step), the renderer aborts the
> current navigation and starts a new navigation to the "unreachable web data
> URL". Internally, it just renders a static HTML error message, but the rest of
> the navigation looks like a normal navigation.
> 
> The old extension code would explicitly check for this "unreachable web data"
> URL. However, that only worked in the browser tests which are executed from
> chrome-extension:// urls. When the tab contents is displaying a http:// url,
the
> URL will be removed by the renderer host due to security policy.
> 
> So instead of checking that URL in the extension api, I check it in the
renderer
> host before the policy check (http://codereview.chromium.org/5254005) pass on
a
> boolean flag whether we navigate to an error page.

Powered by Google App Engine
This is Rietveld 408576698