Add isMainFrame param to WebContents did-fail-load event
#5029
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The failures in CI look the same as the current failures for master: https://travis-ci.org/electron/electron/builds/120710489 Are those expected? Should I not have branched off master? Everything seems a-ok locally (Mac OS X 10.11.4 x64). |
Mr0grog
added a commit
to Mr0grog/nightmare
that referenced
this pull request
Apr 7, 2016
…t succeeds. In this situation, a failure indicates that no page could be loaded at all. For example, a 404 not found response is a success, whereas trying to hit a server that does exist or having the server hang up halfway through a response is a failure. In these failure cases, an object with the following properties is returned: - message: 'navigation error' - code: Chromium's error code describing what went wrong. Note this is *NOT* the HTTP status code. For more details, see https://code.google.com/p/chromium/codesearch#chromium/src/net/base/net_error_list.h - details: A string with additional details about the error. This may be null or an empty string. - url: The URL that failed to load Additionally, successful page loads now return metadata about the request so later actions or user code can take action based on them. For example, someone might want to check the headers or error out if the request returned a 500 server error. The returned object will have the properties: - url: The URL that was loaded - code: The HTTP status code (e.g. 200, 404, 500) - method: The HTTP method used (e.g. "GET", "POST") - referrer: The page that the window was displaying prior to this load or an empty string if this is the first page load. - headers: An object where the property names are the HTTP header names and the values are arrays of the header values (if the same header was sent multiple times, the array will have multiple values). # Implementation Notes Accurately detecting and handling the failure of the main frame turns out to be somewhat complicated. The `did-fail-load` event is always fired whenever a frame (and only a frame) fails, regardless of how it failed. However, if fires for both the main frame and sub frames (iframes, framesets, etc). It currently contains no information about which frame the failure was on (hopefully this will change; see electron/electron#5029), so we use the URL of the failure event to figure this out. However, that leads to its own complexities. First off, if no request was ever made (e.g. the URL was total gobbledegook), there is no URL in the `did-fail-load` event. Second, the actual requested URL may not match the URL that was passed to the `loadURL` method. Chromium cleans up and reformats the URL before making a request. Some examples: - ` http://fake.domain/ ` -> `http://fake.domain/` - `http://///fake.domain/` -> `http://fake.domain/` - `http:fake.domain/` -> `http://fake.domain/` - `http://fake.domain` -> `http://fake.domain/` - `http:// fake.domain` -> `http://%20fake.domain/` To handle both these issues, we hook `webContents.session.webRequest.onBeforeRequest` (http://electron.atom.io/docs/v0.37.4/api/session/#seswebrequestonbeforerequestfilter-listener). If no request is ever made before the failure event, we know the failure must have been for the main frame. If a request is made, we can pick up the URL it is being made to and use that to determine whether later failures are the main frame or for sub-frames. As a happy side effect, this handles redirects, too :) Unfortunately, you can only install one listener at a time for the various `webRequest` methods. Since plugins can now access the Electron process, they could potentially clobber our listener. To mitigate that, we replace the `onBeforeRequest` method with a fake one that registers a listener for *our* code to call from our real `onBeforeRequest` listener. This is far from ideal, but there doesn't seem to be a reliable and safe way around it. (Note: listening to the `did-start-loading` event would be simpler, but it does not include the URL being loaded, so does help us figure out how Chromium may have reformatted the URL).
|
Looks good to me, thanks! |
|
No problem; thanks for merging! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a fairly simple patch to add a
isMainFrameboolean param for thedid-fail-loadevent on WebContents, indicating whether the failed load came from the main frame or from a child frame.Fixes #5013.