From 7d90b44187cfa8f93df6a6341da41cf8192d18ad Mon Sep 17 00:00:00 2001 From: Pierre Rossi Date: Tue, 17 Jun 2014 16:15:35 +0200 Subject: Expose better error information in loadRequest. Use the chromium localized error strings for that purpose, otherwise the error description is always empty. While we're at it, let's tap into the chromium error pages, which should hopefully make sense for most errors, and add some static asserts to check that the qt quick enum and the core one are in sync. Change-Id: Icf8fa7c3bf4a674c60a10950422135fb6930447a Reviewed-by: Andras Becsi --- src/core/chrome_qt.gyp | 44 +++++++++++++++++++++-- src/core/content_client_qt.cpp | 9 +++++ src/core/content_client_qt.h | 1 + src/core/core_gyp_generator.pro | 1 - src/core/qtwebengine.gypi | 3 +- src/core/renderer/content_renderer_client_qt.cpp | 45 ++++++++++++++++++++++++ src/core/renderer/content_renderer_client_qt.h | 5 +-- src/core/resources/grit_action.gypi | 41 +++++++++++++++++++++ src/core/resources/repack_locales.gypi | 1 - src/core/resources/repack_resources.gypi | 2 +- src/core/resources/resources.gyp | 1 + src/core/web_engine_library_info.cpp | 5 +++ src/core/web_engine_library_info.h | 3 ++ src/webengine/api/qquickwebengineview.cpp | 4 +++ tools/buildscripts/repack_locales.py | 9 +++++ tools/scripts/take_snapshot.py | 2 ++ 16 files changed, 167 insertions(+), 9 deletions(-) create mode 100644 src/core/resources/grit_action.gypi diff --git a/src/core/chrome_qt.gyp b/src/core/chrome_qt.gyp index a38b4a9bf..81ab2e881 100644 --- a/src/core/chrome_qt.gyp +++ b/src/core/chrome_qt.gyp @@ -1,18 +1,56 @@ { + 'variables': { + 'grit_out_dir': '<(SHARED_INTERMEDIATE_DIR)/chrome', + }, 'targets': [ { 'target_name': 'chrome_qt', 'type': 'static_library', + 'dependencies': [ + 'chrome_resources', + ], 'include_dirs': [ + './', '<(chromium_src_dir)', '<(chromium_src_dir)/skia/config', + '<(SHARED_INTERMEDIATE_DIR)/chrome', # Needed to include grit-generated files in localized_error.cc ], 'sources': [ - '<(chromium_src_dir)/chrome/browser/media/desktop_streams_registry.h', '<(chromium_src_dir)/chrome/browser/media/desktop_streams_registry.cc', + '<(chromium_src_dir)/chrome/browser/media/desktop_streams_registry.h', '<(chromium_src_dir)/chrome/browser/media/desktop_media_list.h', + '<(chromium_src_dir)/chrome/common/localized_error.cc', + '<(chromium_src_dir)/chrome/common/localized_error.h', + '<(chromium_src_dir)/chrome/common/net/net_error_info.cc', + '<(chromium_src_dir)/chrome/common/net/net_error_info.h', ], - } + }, + { + 'target_name': 'chrome_resources', + 'type': 'none', + 'actions': [ + { + 'action_name': 'generated_resources', + 'variables': { + 'grit_grd_file': '<(chromium_src_dir)/chrome/app/generated_resources.grd', + }, + 'includes': [ 'resources/grit_action.gypi' ], + }, + { + 'action_name': 'chromium_strings.grd', + 'variables': { + 'grit_grd_file': '<(chromium_src_dir)/chrome/app/chromium_strings.grd', + }, + 'includes': [ 'resources/grit_action.gypi' ], + }, + { + 'action_name': 'renderer_resources', + 'variables': { + 'grit_grd_file': '<(chromium_src_dir)/chrome/renderer/resources/renderer_resources.grd', + }, + 'includes': [ 'resources/grit_action.gypi' ], + }, + ] + }, ], } - diff --git a/src/core/content_client_qt.cpp b/src/core/content_client_qt.cpp index 6a52cb5d4..972fd43f7 100644 --- a/src/core/content_client_qt.cpp +++ b/src/core/content_client_qt.cpp @@ -46,6 +46,9 @@ #include "ui/base/l10n/l10n_util.h" #include "ui/base/resource/resource_bundle.h" +#include +#include + base::StringPiece ContentClientQt::GetDataResource(int resource_id, ui::ScaleFactor scale_factor) const { return ResourceBundle::GetSharedInstance().GetRawDataResourceForScale(resource_id, scale_factor); } @@ -54,3 +57,9 @@ base::string16 ContentClientQt::GetLocalizedString(int message_id) const { return l10n_util::GetStringUTF16(message_id); } + +std::string ContentClientQt::GetProduct() const +{ + QString productName(qApp->applicationName() % '/' % qApp->applicationVersion()); + return productName.toStdString(); +} diff --git a/src/core/content_client_qt.h b/src/core/content_client_qt.h index 66f302825..ff4b543ec 100644 --- a/src/core/content_client_qt.h +++ b/src/core/content_client_qt.h @@ -51,6 +51,7 @@ class ContentClientQt : public content::ContentClient { public: virtual base::StringPiece GetDataResource(int, ui::ScaleFactor) const Q_DECL_OVERRIDE; virtual base::string16 GetLocalizedString(int message_id) const Q_DECL_OVERRIDE; + virtual std::string GetProduct() const Q_DECL_OVERRIDE; }; #endif // CONTENT_CLIENT_QT_H diff --git a/src/core/core_gyp_generator.pro b/src/core/core_gyp_generator.pro index 1c2857030..27ee33e51 100644 --- a/src/core/core_gyp_generator.pro +++ b/src/core/core_gyp_generator.pro @@ -78,7 +78,6 @@ SOURCES = \ web_event_factory.cpp \ yuv_video_node.cpp - HEADERS = \ browser_accessibility_manager_qt.h \ browser_accessibility_qt.h \ diff --git a/src/core/qtwebengine.gypi b/src/core/qtwebengine.gypi index c01998c59..d1bee5fb6 100644 --- a/src/core/qtwebengine.gypi +++ b/src/core/qtwebengine.gypi @@ -28,11 +28,12 @@ '<(chromium_src_dir)/v8/tools/gyp/v8.gyp:v8', '<(chromium_src_dir)/webkit/glue/webkit_glue.gyp:*', '<(chromium_src_dir)/third_party/WebKit/Source/web/web.gyp:webkit', - 'chrome_qt.gyp:*', + 'chrome_qt.gyp:chrome_qt', ], 'include_dirs': [ '<(chromium_src_dir)', '<(SHARED_INTERMEDIATE_DIR)/net', # Needed to include grit/net_resources.h + '<(SHARED_INTERMEDIATE_DIR)/chrome', # Needed to include grit/generated_resources.h ], # Chromium code defines those in common.gypi, do the same for our code that include Chromium headers. 'defines': [ diff --git a/src/core/renderer/content_renderer_client_qt.cpp b/src/core/renderer/content_renderer_client_qt.cpp index 0b497a004..ecb469a9d 100644 --- a/src/core/renderer/content_renderer_client_qt.cpp +++ b/src/core/renderer/content_renderer_client_qt.cpp @@ -41,10 +41,55 @@ #include "renderer/content_renderer_client_qt.h" +#include "base/strings/utf_string_conversions.h" +#include "chrome/common/localized_error.h" +#include "content/public/renderer/render_thread.h" +#include "net/base/net_errors.h" +#include "third_party/WebKit/public/platform/WebURLError.h" +#include "third_party/WebKit/public/platform/WebURLRequest.h" +#include "ui/base/resource/resource_bundle.h" +#include "ui/base/webui/jstemplate_builder.h" + #include "renderer/qt_render_view_observer.h" +#include "grit/renderer_resources.h" + +static const char kHttpErrorDomain[] = "http"; + void ContentRendererClientQt::RenderViewCreated(content::RenderView* render_view) { // RenderViewObserver destroys itself with its RenderView. new QtRenderViewObserver(render_view); } + +// To tap into the chromium localized strings. Ripped from the chrome layer (highly simplified). +void ContentRendererClientQt::GetNavigationErrorStrings(blink::WebFrame *frame, const blink::WebURLRequest &failed_request, const blink::WebURLError &error, const std::string &accept_languages, std::string *error_html, base::string16 *error_description) +{ + Q_UNUSED(frame) + + const bool isPost = EqualsASCII(failed_request.httpMethod(), "POST"); + + if (error_html) { + // Use a local error page. + int resource_id; + base::DictionaryValue error_strings; + + const std::string locale = content::RenderThread::Get()->GetLocale(); + // TODO(elproxy): We could potentially get better diagnostics here by first calling NetErrorHelper::GetErrorStringsForDnsProbe + LocalizedError::GetStrings(error.reason, error.domain.utf8(), + error.unreachableURL, isPost, locale, + accept_languages, &error_strings); + resource_id = IDR_NET_ERROR_HTML; + + + const base::StringPiece template_html(ui::ResourceBundle::GetSharedInstance().GetRawDataResource(resource_id)); + if (template_html.empty()) + NOTREACHED() << "unable to load template. ID: " << resource_id; + else // "t" is the id of the templates root node. + *error_html = webui::GetTemplatesHtml(template_html, &error_strings, "t"); + } + + if (error_description) { + *error_description = LocalizedError::GetErrorDetails(error, isPost); + } +} diff --git a/src/core/renderer/content_renderer_client_qt.h b/src/core/renderer/content_renderer_client_qt.h index 100bfd703..360e82f2c 100644 --- a/src/core/renderer/content_renderer_client_qt.h +++ b/src/core/renderer/content_renderer_client_qt.h @@ -47,6 +47,7 @@ class ContentRendererClientQt : public content::ContentRendererClient { public: virtual void RenderViewCreated(content::RenderView *render_view) Q_DECL_OVERRIDE; - // Update this when we want to allow overriding error pages. - virtual bool ShouldSuppressErrorPage(const GURL &) Q_DECL_OVERRIDE { return true; } + virtual bool ShouldSuppressErrorPage(const GURL &) Q_DECL_OVERRIDE { return false; } + virtual void GetNavigationErrorStrings(blink::WebFrame* frame, const blink::WebURLRequest& failed_request, const blink::WebURLError& error + , const std::string& accept_languages, std::string* error_html, base::string16* error_description) Q_DECL_OVERRIDE; }; diff --git a/src/core/resources/grit_action.gypi b/src/core/resources/grit_action.gypi new file mode 100644 index 000000000..e8b98b43d --- /dev/null +++ b/src/core/resources/grit_action.gypi @@ -0,0 +1,41 @@ +# Copyright (c) 2011 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +# This file is meant to be included into an action to invoke grit in a +# consistent manner. To use this the following variables need to be +# defined: +# grit_grd_file: string: grd file path +# grit_out_dir: string: the output directory path + +# It would be really nice to do this with a rule instead of actions, but it +# would need to determine inputs and outputs via grit_info on a per-file +# basis. GYP rules don’t currently support that. They could be extended to +# do this, but then every generator would need to be updated to handle this. + +{ + 'variables': { + 'grit_cmd': ['python', '<(DEPTH)/tools/grit/grit.py'], + 'grit_resource_ids%': '<(DEPTH)/tools/gritsettings/resource_ids', + # This makes it possible to add more defines in specific targets, + # instead of build/common.gypi . + 'grit_additional_defines%': [], + }, + 'inputs': [ + 'applicationName()); +} diff --git a/src/core/web_engine_library_info.h b/src/core/web_engine_library_info.h index 84fee12af..4ce760dc4 100644 --- a/src/core/web_engine_library_info.h +++ b/src/core/web_engine_library_info.h @@ -43,6 +43,7 @@ #define WEB_ENGINE_LIBRARY_INFO_H #include "base/files/file_path.h" +#include "base/strings/string16.h" enum { QT_RESOURCES_PAK = 5000 }; @@ -50,6 +51,8 @@ enum { class WebEngineLibraryInfo { public: static base::FilePath getPath(int key); + // Called by localized_error in our custom chrome layer + static base::string16 getApplicationName(); }; diff --git a/src/webengine/api/qquickwebengineview.cpp b/src/webengine/api/qquickwebengineview.cpp index ad57f24ba..a6a11c07d 100644 --- a/src/webengine/api/qquickwebengineview.cpp +++ b/src/webengine/api/qquickwebengineview.cpp @@ -282,6 +282,10 @@ void QQuickWebEngineViewPrivate::loadVisuallyCommitted() Q_EMIT e->loadVisuallyCommitted(); } +Q_STATIC_ASSERT(static_cast(WebEngineError::NoErrorDomain) == static_cast(QQuickWebEngineView::NoErrorDomain)); +Q_STATIC_ASSERT(static_cast(WebEngineError::CertificateErrorDomain) == static_cast(QQuickWebEngineView::CertificateErrorDomain)); +Q_STATIC_ASSERT(static_cast(WebEngineError::DnsErrorDomain) == static_cast(QQuickWebEngineView::DnsErrorDomain)); + void QQuickWebEngineViewPrivate::loadFinished(bool success, int error_code, const QString &error_description) { Q_Q(QQuickWebEngineView); diff --git a/tools/buildscripts/repack_locales.py b/tools/buildscripts/repack_locales.py index eabc04e9f..05e2895ed 100755 --- a/tools/buildscripts/repack_locales.py +++ b/tools/buildscripts/repack_locales.py @@ -105,6 +105,15 @@ def calc_inputs(locale): inputs.append(os.path.join(SHARE_INT_DIR, 'ui', 'app_locale_settings', 'app_locale_settings_%s.pak' % locale)) + # Used for localized error messages and error pages + #e.g. '<(SHARED_INTERMEDIATE_DIR)/chrome/generated_resources_da.pak' + inputs.append(os.path.join(SHARE_INT_DIR, 'chrome', + 'generated_resources_%s.pak' % locale)) + + #e.g. '<(SHARED_INTERMEDIATE_DIR)/chrome/generated_resources_da.pak' + inputs.append(os.path.join(SHARE_INT_DIR, 'chrome', + 'chromium_strings_%s.pak' % locale)) + # Add any extra input files. for extra_file in EXTRA_INPUT_FILES: inputs.append('%s_%s.pak' % (extra_file, locale)) diff --git a/tools/scripts/take_snapshot.py b/tools/scripts/take_snapshot.py index bda2a966f..d6b8fedb1 100755 --- a/tools/scripts/take_snapshot.py +++ b/tools/scripts/take_snapshot.py @@ -100,6 +100,8 @@ def isInChromiumBlacklist(file_path): not 'media/desktop_media_list.h' in file_path and not 'media/desktop_streams_registry.cc' in file_path and not 'media/desktop_streams_registry.h' in file_path and + not 'net/net_error_info' in file_path and + not 'common/localized_error' in file_path and not file_path.endswith('.isolate') and not file_path.endswith('cf_resources.rc') and not file_path.endswith('version.py') and -- cgit v1.2.3