From 919dce4400651813d5ff6e8a85b944a5987adcb7 Mon Sep 17 00:00:00 2001 From: Bartosz Fabianowski Date: Thu, 16 Apr 2015 12:10:58 +0200 Subject: Speculative revert by sheriff I am reverting all CLs that could have caused this failure: http://build.chromium.org/p/chromium.linux/builders/ Android%20Tests%20%28dbg%29/builds/27276 I will start re-landing vindicated CLs shortly. Revert "Convert a11y_page.css to a11y_page_style.html. See http://goo.gl/vIGSCO for more information." This reverts commit eb48d65705e44b80d1d7a3b4f21c567802c102bb. Revert "Supply build properties to run-bisect-perf regression for android bisects." This reverts commit 9d70bb94984b02c17678ccf482d1a7b7c760a43f. Revert "[cros New-GAIA] Webview login and new GAIA endpoint enabled by default" This reverts commit 29061783f8b59f1e78286a87a3bb7d18129f639a. Revert "Prime the landing pad for the new video rendering pipeline." This reverts commit 45a3c93f745eabf6c1b1cbdac87ed4350a919e76. Revert "Implement separate error dialog for cws widget container" This reverts commit 7ed1f876f6f4d79ac074b25f2066d514603aa18d. Revert "Create device_bluetooth watchlist; add scheib." This reverts commit 40514c99cd45d1c8e9caef3245d42533d9810179. Revert "Convert checkbox.css to checkbox_style.html. See http://goo.gl/vIGSCO for more information." This reverts commit a687f31a786645508622482e9033f497364a43cd. Revert "Created new URLRequestContext for secure proxy check." This reverts commit 652eabf1141c00594aa6e9ed81beb980aec89198. Revert "Revert of Fix scroll regression when specifying an extension id. (patchset #4 id:60001 of https://codereview.chromium.org/1064573003/)" This reverts commit 488846cd562f444831982d82ef0bc9ca0dd79028. Revert "Sync: Add Android test for downloading a bookmark" This reverts commit 9c052713563111ac3d6cc64d4b5cfdfa27c202f4. Revert "ScreenOrientationController to start observing even without an internal display." This reverts commit 31fed68a524d49f99cebf02dca6ad49019e9900c. Revert "Fix MB configurations for Mac and Win GN bots." This reverts commit 31a66f3e705d20a39033a7b7d7f09a60517e6b9d. Revert "Add tool/perf/measurements path to check for CQ jobs." This reverts commit 99e1ac1d217174dad7a862f6a0fc44950d503737. Revert "Implement Notification.data for persistent notifications." This reverts commit 539f51d6af62097059c34b1c19dfbc4352413ad7. Revert "Fix playback rate calculations for WallClockTimeSource." This reverts commit 3ec02d642465872d9ab7d7db600d0480a57b3cab. Revert "Roll src/third_party/pdfium eddab44:b330016" This reverts commit 81d51e253cfacd5648cca9f12b605e63971a9ab1. Revert "cc: Make DisplayItemList::Append replay into an SkPicture" This reverts commit ec7c07e57bc5f91e3ba0bdeba8b6f534978615b0. Revert "Clean up URLFetcher unit tests, part 5." This reverts commit 8eeb3bf4a02be612cacb3cdbb5c223d909939527. BUG=None TBR=akuegel Review URL: https://codereview.chromium.org/1083683003 Cr-Commit-Position: refs/heads/master@{#325411} --- BUILD.gn | 1 + DEPS | 2 +- WATCHLISTS | 4 - .../screen_orientation_controller_chromeos.cc | 22 +- ...een_orientation_controller_chromeos_unittest.cc | 19 -- build/android/buildbot/bb_host_steps.py | 5 +- build/gn_migration.gypi | 1 + cc/blink/web_compositor_support_impl.cc | 4 + cc/blink/web_compositor_support_impl.h | 1 + cc/blink/web_content_layer_impl.cc | 9 +- cc/blink/web_content_layer_impl.h | 3 +- cc/blink/web_display_item_list_impl.cc | 9 +- cc/blink/web_display_item_list_impl.h | 6 +- cc/debug/rasterize_and_record_benchmark.cc | 7 +- cc/layers/content_layer_client.h | 3 +- cc/layers/picture_image_layer.cc | 8 +- cc/layers/picture_image_layer.h | 3 +- cc/layers/picture_image_layer_unittest.cc | 39 +-- cc/layers/picture_layer_unittest.cc | 4 +- cc/layers/video_frame_provider.h | 70 ++--- cc/layers/video_frame_provider_client_impl.cc | 15 +- cc/layers/video_frame_provider_client_impl.h | 4 +- cc/layers/video_layer_impl.cc | 2 +- cc/resources/display_item.cc | 4 + cc/resources/display_item.h | 1 + cc/resources/display_item_list.cc | 63 ++--- cc/resources/display_item_list.h | 14 +- cc/resources/display_item_list_unittest.cc | 40 +-- cc/resources/display_list_recording_source.cc | 13 +- cc/resources/drawing_display_item.cc | 11 + cc/resources/drawing_display_item.h | 1 + cc/test/fake_content_layer_client.cc | 21 +- cc/test/fake_content_layer_client.h | 3 +- cc/test/fake_display_list_recording_source.h | 8 +- cc/test/fake_video_frame_provider.cc | 5 - cc/test/fake_video_frame_provider.h | 4 +- cc/test/solid_color_content_layer_client.cc | 5 +- cc/test/solid_color_content_layer_client.h | 3 +- cc/trees/layer_tree_host_common_unittest.cc | 4 +- cc/trees/layer_tree_host_pixeltest_masks.cc | 12 +- cc/trees/layer_tree_host_unittest.cc | 8 +- .../chrome/browser/sync/FakeServerHelper.java | 52 +--- .../src/chromium/chrome/browser/sync/SyncTest.java | 17 -- chrome/browser/chromeos/login/helper.cc | 16 +- .../chromeos/login/hid_detection_browsertest.cc | 16 +- chrome/browser/chromeos/login/login_browsertest.cc | 35 +-- .../browser/chromeos/login/login_manager_test.cc | 32 +-- chrome/browser/chromeos/login/login_manager_test.h | 7 - chrome/browser/chromeos/login/oobe_browsertest.cc | 66 +++-- .../login/proxy_auth_dialog_browsertest.cc | 20 +- .../chromeos/login/saml/saml_browsertest.cc | 15 +- .../chromeos/login/session/user_session_manager.cc | 52 +--- .../chromeos/login/signin/oauth2_browsertest.cc | 44 +-- chrome/browser/chromeos/login/startup_utils.cc | 37 +-- .../browser/chromeos/login/test/oobe_base_test.cc | 74 ++--- .../browser/chromeos/login/test/oobe_base_test.h | 25 -- .../chromeos/login/webview_login_browsertest.cc | 44 ++- .../login/wizard_controller_browsertest.cc | 41 +-- .../chromeos/policy/blocking_login_browsertest.cc | 37 ++- .../browser/resources/extensions/extension_list.js | 29 +- chrome/browser/resources/extensions/extensions.js | 1 - .../resources/settings/a11y_page/a11y_page.css | 23 ++ .../resources/settings/a11y_page/a11y_page.html | 3 +- .../settings/a11y_page/a11y_page_style.html | 24 -- .../resources/settings/checkbox/checkbox.css | 19 ++ .../resources/settings/checkbox/checkbox.html | 3 +- .../settings/checkbox/checkbox_style.html | 20 -- .../resources/settings/settings_resources.grd | 8 +- chrome/chrome_tests.gypi | 4 - chrome/common/pref_names.cc | 5 +- chrome/common/pref_names.h | 2 +- chrome/interactive_ui_tests.isolate | 1 - .../platform_notification_service.html | 5 +- chromecast/media/base/switching_media_renderer.cc | 3 +- chromecast/media/base/switching_media_renderer.h | 1 + chromecast/media/cma/filters/cma_renderer.cc | 13 +- chromecast/media/cma/filters/cma_renderer.h | 7 +- .../media/chromecast_media_renderer_factory.cc | 8 +- .../media/chromecast_media_renderer_factory.h | 3 +- .../core/browser/data_reduction_proxy_config.cc | 146 +++------- .../core/browser/data_reduction_proxy_config.h | 35 +-- .../data_reduction_proxy_config_test_utils.cc | 1 + .../data_reduction_proxy_config_test_utils.h | 3 - .../data_reduction_proxy_config_unittest.cc | 9 +- .../core/browser/data_reduction_proxy_io_data.cc | 11 +- .../core/browser/data_reduction_proxy_service.cc | 46 +++ .../core/browser/data_reduction_proxy_service.h | 30 +- .../data_reduction_proxy_settings_unittest.cc | 12 - .../core/browser/data_reduction_proxy_test_utils.h | 3 + .../notifications/notification_database_data.proto | 1 - .../notification_database_data_conversions.cc | 10 - .../notification_database_data_unittest.cc | 9 - .../notifications/notification_data_conversions.cc | 2 - .../notification_data_conversions_unittest.cc | 24 +- .../child/notifications/notification_manager.cc | 21 -- content/common/platform_notification_messages.h | 1 - .../public/common/platform_notification_data.cc | 4 +- content/public/common/platform_notification_data.h | 13 +- .../media/android/webmediaplayer_android.cc | 9 +- .../media/android/webmediaplayer_android.h | 4 +- content/renderer/media/webmediaplayer_ms.cc | 11 +- content/renderer/media/webmediaplayer_ms.h | 4 +- media/BUILD.gn | 31 ++ media/base/mock_filters.h | 24 +- media/base/pipeline.cc | 4 + media/base/pipeline.h | 4 + media/base/pipeline_unittest.cc | 13 +- media/base/renderer.h | 4 + media/base/renderer_factory.h | 7 +- media/base/video_renderer.h | 4 + media/base/video_renderer_sink.h | 64 ----- media/base/wall_clock_time_source.cc | 2 +- media/base/wall_clock_time_source_unittest.cc | 28 +- media/blink/video_frame_compositor.cc | 89 +----- media/blink/video_frame_compositor.h | 78 +----- media/blink/video_frame_compositor_unittest.cc | 27 +- media/blink/webmediaplayer_impl.cc | 27 +- media/media.gyp | 38 +++ media/mojo/services/mojo_renderer_factory.cc | 3 +- media/mojo/services/mojo_renderer_factory.h | 3 +- media/mojo/services/mojo_renderer_impl.cc | 4 +- media/mojo/services/mojo_renderer_impl.h | 1 + media/mojo/services/mojo_renderer_service.cc | 8 +- media/mojo/services/mojo_renderer_service.h | 2 - media/mojo/services/renderer_config.cc | 4 - media/mojo/services/renderer_config.h | 6 +- media/mojo/services/renderer_config_default.cc | 18 -- media/renderers/default_renderer_factory.cc | 8 +- media/renderers/default_renderer_factory.h | 4 +- media/renderers/renderer_impl.cc | 4 + media/renderers/renderer_impl.h | 2 + media/renderers/renderer_impl_unittest.cc | 11 +- media/renderers/video_renderer_impl.cc | 20 +- media/renderers/video_renderer_impl.h | 11 +- media/renderers/video_renderer_impl_unittest.cc | 45 ++- media/test/pipeline_integration_test.cc | 10 +- media/test/pipeline_integration_test_base.cc | 12 +- media/test/pipeline_integration_test_base.h | 15 - media/tools/player_x11/data_source_logger.cc | 59 ++++ media/tools/player_x11/data_source_logger.h | 41 +++ media/tools/player_x11/gl_video_renderer.cc | 251 +++++++++++++++++ media/tools/player_x11/gl_video_renderer.h | 43 +++ media/tools/player_x11/player_x11.cc | 311 +++++++++++++++++++++ media/tools/player_x11/x11_video_renderer.cc | 215 ++++++++++++++ media/tools/player_x11/x11_video_renderer.h | 47 ++++ net/url_request/url_fetcher_impl_unittest.cc | 280 ++++++++++++------- net/url_request/url_request_test_util.h | 3 +- sync/BUILD.gn | 1 - .../android/fake_server_helper_android.cc | 41 --- .../android/fake_server_helper_android.h | 15 - sync/test/fake_server/bookmark_entity_builder.cc | 13 +- sync/test/fake_server/bookmark_entity_builder.h | 7 - sync/test/fake_server/fake_server.cc | 62 ++-- sync/test/fake_server/fake_server.h | 10 +- tools/mb/mb_config.pyl | 14 +- tools/metrics/histograms/histograms.xml | 9 - tools/run-bisect-perf-regression.py | 4 +- ui/compositor/layer.cc | 7 +- ui/compositor/layer.h | 3 +- .../foreground/css/cws_widget_container.css | 17 -- .../foreground/js/compiled_resources.gyp | 1 - .../foreground/js/cws_widget_container.js | 15 +- .../file_manager/foreground/js/main_scripts.js | 1 - .../js/ui/cws_widget_container_error_dialog.js | 58 ---- ui/file_manager/file_manager/main.html | 1 - 165 files changed, 2026 insertions(+), 1820 deletions(-) create mode 100644 chrome/browser/resources/settings/a11y_page/a11y_page.css delete mode 100644 chrome/browser/resources/settings/a11y_page/a11y_page_style.html create mode 100644 chrome/browser/resources/settings/checkbox/checkbox.css delete mode 100644 chrome/browser/resources/settings/checkbox/checkbox_style.html delete mode 100644 media/base/video_renderer_sink.h create mode 100644 media/tools/player_x11/data_source_logger.cc create mode 100644 media/tools/player_x11/data_source_logger.h create mode 100644 media/tools/player_x11/gl_video_renderer.cc create mode 100644 media/tools/player_x11/gl_video_renderer.h create mode 100644 media/tools/player_x11/player_x11.cc create mode 100644 media/tools/player_x11/x11_video_renderer.cc create mode 100644 media/tools/player_x11/x11_video_renderer.h delete mode 100644 ui/file_manager/file_manager/foreground/js/ui/cws_widget_container_error_dialog.js diff --git a/BUILD.gn b/BUILD.gn index b51ff78..cf1bd2b 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -469,6 +469,7 @@ group("gn_all") { } if (use_x11) { + deps += [ "//media:player_x11" ] if (target_cpu != "arm") { deps += [ "//gpu/tools/compositor_model_bench" ] } diff --git a/DEPS b/DEPS index b5c6cda..199d9d0 100644 --- a/DEPS +++ b/DEPS @@ -66,7 +66,7 @@ vars = { # Three lines of non-changing comments so that # the commit queue can handle CLs rolling PDFium # and whatever else without interference from each other. - 'pdfium_revision': 'b3300162a1ebacc973ff9793029caf4db9a4f5e5', + 'pdfium_revision': 'eddab4425614e49146f904f00da4a664ba4b581b', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling openmax_dl # and whatever else without interference from each other. diff --git a/WATCHLISTS b/WATCHLISTS index f0239b0..a07fcca 100644 --- a/WATCHLISTS +++ b/WATCHLISTS @@ -274,9 +274,6 @@ 'deep_memory_profiler': { 'filepath': 'tools/(deep_memory_profiler|find_runtime_symbols)', }, - 'device_bluetooth': { - 'filepath': 'device/.*bluetooth' - }, 'device_sensors': { 'filepath': 'content/browser/device_sensors/|'\ 'content/common/device_sensors/|'\ @@ -928,7 +925,6 @@ 'cookie_monster': ['erikwright@chromium.org'], 'custom_handlers': ['vabr+watchlist@chromium.org'], 'deep_memory_profiler': ['dmikurube@chromium.org'], - 'device_bluetooth': ['scheib+watch@chromium.org'], 'device_sensors': ['timvolodine@chromium.org', 'mvanouwerkerk@chromium.org', 'rijubrata.bhaumik@intel.com', diff --git a/ash/content/display/screen_orientation_controller_chromeos.cc b/ash/content/display/screen_orientation_controller_chromeos.cc index dbd4465..07659d7c 100644 --- a/ash/content/display/screen_orientation_controller_chromeos.cc +++ b/ash/content/display/screen_orientation_controller_chromeos.cc @@ -196,11 +196,10 @@ void ScreenOrientationController::Unlock(content::WebContents* web_contents) { void ScreenOrientationController::OnDisplayConfigurationChanged() { if (ignore_display_configuration_updates_) return; - DisplayManager* display_manager = Shell::GetInstance()->display_manager(); - if (!display_manager->HasInternalDisplay()) - return; gfx::Display::Rotation user_rotation = - display_manager->GetDisplayInfo(gfx::Display::InternalDisplayId()) + Shell::GetInstance() + ->display_manager() + ->GetDisplayInfo(gfx::Display::InternalDisplayId()) .rotation(); if (user_rotation != current_rotation_) { // A user may change other display configuration settings. When the user @@ -213,14 +212,11 @@ void ScreenOrientationController::OnDisplayConfigurationChanged() { void ScreenOrientationController::OnMaximizeModeStarted() { DisplayManager* display_manager = Shell::GetInstance()->display_manager(); - // Do not exit early, as the internal display can be determined after Maximize - // Mode has started. (chrome-os-partner:38796) - // Always start observing. - if (display_manager->HasInternalDisplay()) { - current_rotation_ = user_rotation_ = - display_manager->GetDisplayInfo(gfx::Display::InternalDisplayId()) - .rotation(); - } + if (!display_manager->HasInternalDisplay()) + return; + current_rotation_ = user_rotation_ = + display_manager->GetDisplayInfo(gfx::Display::InternalDisplayId()) + .rotation(); if (!rotation_locked_) LoadDisplayRotationProperties(); chromeos::AccelerometerReader::GetInstance()->AddObserver(this); @@ -228,6 +224,8 @@ void ScreenOrientationController::OnMaximizeModeStarted() { } void ScreenOrientationController::OnMaximizeModeEnded() { + if (!Shell::GetInstance()->display_manager()->HasInternalDisplay()) + return; chromeos::AccelerometerReader::GetInstance()->RemoveObserver(this); Shell::GetInstance()->display_controller()->RemoveObserver(this); if (current_rotation_ != user_rotation_) diff --git a/ash/content/display/screen_orientation_controller_chromeos_unittest.cc b/ash/content/display/screen_orientation_controller_chromeos_unittest.cc index 342e958..ca02ca3 100644 --- a/ash/content/display/screen_orientation_controller_chromeos_unittest.cc +++ b/ash/content/display/screen_orientation_controller_chromeos_unittest.cc @@ -589,23 +589,4 @@ TEST_F(ScreenOrientationControllerTest, UserRotationLockDisallowsRotation) { EXPECT_EQ(gfx::Display::ROTATE_0, GetInternalDisplayRotation()); } -// Tests that when MaximizeMode is triggered before the internal display is -// ready, that ScreenOrientationController still begins listening to events, -// which require an internal display to be acted upon. -TEST_F(ScreenOrientationControllerTest, InternalDisplayNotAvailableAtStartup) { - int64 internal_display_id = gfx::Display::InternalDisplayId(); - gfx::Display::SetInternalDisplayId(gfx::Display::kInvalidDisplayID); - - EnableMaximizeMode(true); - - // Should not crash, even thought there is no internal display. - SetInternalDisplayRotation(gfx::Display::ROTATE_180); - EXPECT_FALSE(RotationLocked()); - - // With an internal display now available, functionality should resume. - gfx::Display::SetInternalDisplayId(internal_display_id); - SetInternalDisplayRotation(gfx::Display::ROTATE_90); - EXPECT_TRUE(RotationLocked()); -} - } // namespace ash diff --git a/build/android/buildbot/bb_host_steps.py b/build/android/buildbot/bb_host_steps.py index 1e927fb..6630321 100755 --- a/build/android/buildbot/bb_host_steps.py +++ b/build/android/buildbot/bb_host_steps.py @@ -4,7 +4,6 @@ # found in the LICENSE file. import os -import json import sys import bb_utils @@ -86,9 +85,7 @@ def BisectPerfRegression(options): RunCmd([SrcPath('tools', 'prepare-bisect-perf-regression.py'), '-w', os.path.join(constants.DIR_SOURCE_ROOT, os.pardir)]) RunCmd([SrcPath('tools', 'run-bisect-perf-regression.py'), - '-w', os.path.join(constants.DIR_SOURCE_ROOT, os.pardir), - '--build-properties=%s' % json.dumps(options.build_properties)] + - args) + '-w', os.path.join(constants.DIR_SOURCE_ROOT, os.pardir)] + args) def GetHostStepCmds(): diff --git a/build/gn_migration.gypi b/build/gn_migration.gypi index ad6560c..1dacbb5 100644 --- a/build/gn_migration.gypi +++ b/build/gn_migration.gypi @@ -281,6 +281,7 @@ ['use_x11==1', { 'dependencies': [ + '../media/media.gyp:player_x11', '../tools/xdisplaycheck/xdisplaycheck.gyp:xdisplaycheck', ], 'conditions': [ diff --git a/cc/blink/web_compositor_support_impl.cc b/cc/blink/web_compositor_support_impl.cc index e69abf6..e335ba3 100644 --- a/cc/blink/web_compositor_support_impl.cc +++ b/cc/blink/web_compositor_support_impl.cc @@ -93,6 +93,10 @@ WebScrollbarLayer* WebCompositorSupportImpl::createSolidColorScrollbarLayer( is_left_side_vertical_scrollbar); } +WebDisplayItemList* WebCompositorSupportImpl::createDisplayItemList() { + return new WebDisplayItemListImpl(); +} + WebCompositorAnimation* WebCompositorSupportImpl::createAnimation( const blink::WebCompositorAnimationCurve& curve, blink::WebCompositorAnimation::TargetProperty target, diff --git a/cc/blink/web_compositor_support_impl.h b/cc/blink/web_compositor_support_impl.h index b924be9..4b41691 100644 --- a/cc/blink/web_compositor_support_impl.h +++ b/cc/blink/web_compositor_support_impl.h @@ -42,6 +42,7 @@ class CC_BLINK_EXPORT WebCompositorSupportImpl int thumb_thickness, int track_start, bool is_left_side_vertical_scrollbar); + virtual blink::WebDisplayItemList* createDisplayItemList(); virtual blink::WebCompositorAnimation* createAnimation( const blink::WebCompositorAnimationCurve& curve, blink::WebCompositorAnimation::TargetProperty target, diff --git a/cc/blink/web_content_layer_impl.cc b/cc/blink/web_content_layer_impl.cc index 90d7f9b..92242230 100644 --- a/cc/blink/web_content_layer_impl.cc +++ b/cc/blink/web_content_layer_impl.cc @@ -72,15 +72,16 @@ void WebContentLayerImpl::PaintContents( client_->paintContents(canvas, clip, PaintingControlToWeb(painting_control)); } -void WebContentLayerImpl::PaintContentsToDisplayList( - cc::DisplayItemList* display_list, +scoped_refptr +WebContentLayerImpl::PaintContentsToDisplayList( const gfx::Rect& clip, cc::ContentLayerClient::PaintingControlSetting painting_control) { if (!client_) - return; + return cc::DisplayItemList::Create(); - WebDisplayItemListImpl list(display_list); + WebDisplayItemListImpl list; client_->paintContents(&list, clip, PaintingControlToWeb(painting_control)); + return list.ToDisplayItemList(); } bool WebContentLayerImpl::FillsBoundsCompletely() const { diff --git a/cc/blink/web_content_layer_impl.h b/cc/blink/web_content_layer_impl.h index e7b04b0..3e7b55c 100644 --- a/cc/blink/web_content_layer_impl.h +++ b/cc/blink/web_content_layer_impl.h @@ -39,8 +39,7 @@ class WebContentLayerImpl : public blink::WebContentLayer, void PaintContents(SkCanvas* canvas, const gfx::Rect& clip, PaintingControlSetting painting_control) override; - void PaintContentsToDisplayList( - cc::DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting painting_control) override; bool FillsBoundsCompletely() const override; diff --git a/cc/blink/web_display_item_list_impl.cc b/cc/blink/web_display_item_list_impl.cc index 2bad804..a253beb 100644 --- a/cc/blink/web_display_item_list_impl.cc +++ b/cc/blink/web_display_item_list_impl.cc @@ -25,9 +25,12 @@ namespace cc_blink { -WebDisplayItemListImpl::WebDisplayItemListImpl( - cc::DisplayItemList* display_list) - : display_item_list_(display_list) { +WebDisplayItemListImpl::WebDisplayItemListImpl() + : display_item_list_(cc::DisplayItemList::Create()) { +} + +scoped_refptr WebDisplayItemListImpl::ToDisplayItemList() { + return display_item_list_; } void WebDisplayItemListImpl::appendDrawingItem(const SkPicture* picture) { diff --git a/cc/blink/web_display_item_list_impl.h b/cc/blink/web_display_item_list_impl.h index ff94ee3..abaf221 100644 --- a/cc/blink/web_display_item_list_impl.h +++ b/cc/blink/web_display_item_list_impl.h @@ -31,9 +31,11 @@ namespace cc_blink { class WebDisplayItemListImpl : public blink::WebDisplayItemList { public: - CC_BLINK_EXPORT WebDisplayItemListImpl(cc::DisplayItemList* display_list); + CC_BLINK_EXPORT WebDisplayItemListImpl(); virtual ~WebDisplayItemListImpl(); + scoped_refptr ToDisplayItemList(); + // blink::WebDisplayItemList implementation. virtual void appendDrawingItem(const SkPicture*); virtual void appendClipItem( @@ -61,7 +63,7 @@ class WebDisplayItemListImpl : public blink::WebDisplayItemList { virtual void appendEndScrollItem(); private: - cc::DisplayItemList* display_item_list_; + scoped_refptr display_item_list_; DISALLOW_COPY_AND_ASSIGN(WebDisplayItemListImpl); }; diff --git a/cc/debug/rasterize_and_record_benchmark.cc b/cc/debug/rasterize_and_record_benchmark.cc index f9a48c9..ab05f30 100644 --- a/cc/debug/rasterize_and_record_benchmark.cc +++ b/cc/debug/rasterize_and_record_benchmark.cc @@ -206,11 +206,8 @@ void RasterizeAndRecordBenchmark::RunOnDisplayListLayer( kTimeCheckInterval); do { - const bool use_cached_picture = true; - display_list = - DisplayItemList::Create(visible_layer_rect, use_cached_picture); - painter->PaintContentsToDisplayList( - display_list.get(), visible_layer_rect, painting_control); + display_list = painter->PaintContentsToDisplayList(visible_layer_rect, + painting_control); display_list->CreateAndCacheSkPicture(); if (memory_used) { diff --git a/cc/layers/content_layer_client.h b/cc/layers/content_layer_client.h index 3a82136..4f9c754 100644 --- a/cc/layers/content_layer_client.h +++ b/cc/layers/content_layer_client.h @@ -29,8 +29,7 @@ class CC_EXPORT ContentLayerClient { const gfx::Rect& clip, PaintingControlSetting painting_status) = 0; - virtual void PaintContentsToDisplayList( - DisplayItemList* display_list, + virtual scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting painting_status) = 0; diff --git a/cc/layers/picture_image_layer.cc b/cc/layers/picture_image_layer.cc index 2d338f0..b972ec1 100644 --- a/cc/layers/picture_image_layer.cc +++ b/cc/layers/picture_image_layer.cc @@ -63,17 +63,19 @@ void PictureImageLayer::PaintContents( canvas->drawBitmap(bitmap_, 0, 0); } -void PictureImageLayer::PaintContentsToDisplayList( - DisplayItemList* display_list, +scoped_refptr PictureImageLayer::PaintContentsToDisplayList( const gfx::Rect& clip, ContentLayerClient::PaintingControlSetting painting_control) { + scoped_refptr display_item_list = DisplayItemList::Create(); + SkPictureRecorder recorder; SkCanvas* canvas = recorder.beginRecording(gfx::RectToSkRect(clip)); PaintContents(canvas, clip, painting_control); skia::RefPtr picture = skia::AdoptRef(recorder.endRecordingAsPicture()); - display_list->AppendItem(DrawingDisplayItem::Create(picture)); + display_item_list->AppendItem(DrawingDisplayItem::Create(picture)); + return display_item_list; } bool PictureImageLayer::FillsBoundsCompletely() const { diff --git a/cc/layers/picture_image_layer.h b/cc/layers/picture_image_layer.h index a12a1b9..8d40550 100644 --- a/cc/layers/picture_image_layer.h +++ b/cc/layers/picture_image_layer.h @@ -27,8 +27,7 @@ class CC_EXPORT PictureImageLayer : public PictureLayer, ContentLayerClient { SkCanvas* canvas, const gfx::Rect& clip, ContentLayerClient::PaintingControlSetting painting_control) override; - void PaintContentsToDisplayList( - DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, ContentLayerClient::PaintingControlSetting painting_control) override; bool FillsBoundsCompletely() const override; diff --git a/cc/layers/picture_image_layer_unittest.cc b/cc/layers/picture_image_layer_unittest.cc index cd95c9f..5b9375d 100644 --- a/cc/layers/picture_image_layer_unittest.cc +++ b/cc/layers/picture_image_layer_unittest.cc @@ -33,44 +33,9 @@ TEST(PictureImageLayerTest, PaintContentsToDisplayList) { layer->SetBitmap(image_bitmap); layer->SetBounds(gfx::Size(layer_rect.width(), layer_rect.height())); - bool use_cached_picture = false; scoped_refptr display_list = - DisplayItemList::Create(layer_rect, use_cached_picture); - layer->PaintContentsToDisplayList( - display_list.get(), layer_rect, - ContentLayerClient::PAINTING_BEHAVIOR_NORMAL); - unsigned char actual_pixels[4 * 200 * 200] = {0}; - DrawDisplayList(actual_pixels, layer_rect, display_list); - - EXPECT_EQ(0, memcmp(actual_pixels, image_pixels, 4 * 200 * 200)); -} - -TEST(PictureImageLayerTest, PaintContentsToCachedDisplayList) { - scoped_refptr layer = PictureImageLayer::Create(); - gfx::Rect layer_rect(200, 200); - - SkBitmap image_bitmap; - unsigned char image_pixels[4 * 200 * 200] = {0}; - SkImageInfo info = - SkImageInfo::MakeN32Premul(layer_rect.width(), layer_rect.height()); - image_bitmap.installPixels(info, image_pixels, info.minRowBytes()); - SkCanvas image_canvas(image_bitmap); - image_canvas.clear(SK_ColorRED); - SkPaint blue_paint; - blue_paint.setColor(SK_ColorBLUE); - image_canvas.drawRectCoords(0.f, 0.f, 100.f, 100.f, blue_paint); - image_canvas.drawRectCoords(100.f, 100.f, 200.f, 200.f, blue_paint); - - layer->SetBitmap(image_bitmap); - layer->SetBounds(gfx::Size(layer_rect.width(), layer_rect.height())); - - bool use_cached_picture = true; - scoped_refptr display_list = - DisplayItemList::Create(layer_rect, use_cached_picture); - layer->PaintContentsToDisplayList( - display_list.get(), layer_rect, - ContentLayerClient::PAINTING_BEHAVIOR_NORMAL); - display_list->CreateAndCacheSkPicture(); + layer->PaintContentsToDisplayList( + layer_rect, ContentLayerClient::PAINTING_BEHAVIOR_NORMAL); unsigned char actual_pixels[4 * 200 * 200] = {0}; DrawDisplayList(actual_pixels, layer_rect, display_list); diff --git a/cc/layers/picture_layer_unittest.cc b/cc/layers/picture_layer_unittest.cc index 917a36f..2f64693 100644 --- a/cc/layers/picture_layer_unittest.cc +++ b/cc/layers/picture_layer_unittest.cc @@ -25,11 +25,11 @@ class MockContentLayerClient : public ContentLayerClient { void PaintContents(SkCanvas* canvas, const gfx::Rect& clip, PaintingControlSetting picture_control) override {} - void PaintContentsToDisplayList( - DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting picture_control) override { NOTIMPLEMENTED(); + return DisplayItemList::Create(); } bool FillsBoundsCompletely() const override { return false; }; }; diff --git a/cc/layers/video_frame_provider.h b/cc/layers/video_frame_provider.h index ff632f6..45e6c411 100644 --- a/cc/layers/video_frame_provider.h +++ b/cc/layers/video_frame_provider.h @@ -6,7 +6,6 @@ #define CC_LAYERS_VIDEO_FRAME_PROVIDER_H_ #include "base/memory/ref_counted.h" -#include "base/time/time.h" #include "cc/base/cc_export.h" namespace media { @@ -15,39 +14,27 @@ class VideoFrame; namespace cc { -// VideoFrameProvider and VideoFrameProvider::Client define the relationship by -// which video frames are exchanged between a provider and client. -// -// Threading notes: This class may be used in a multithreaded manner. However, -// if the Client implementation calls GetCurrentFrame()/PutCurrentFrame() from -// one thread, the provider must ensure that all client methods (except -// StopUsingProvider()) are called from that thread (typically the compositor -// thread). +// Threading notes: This class may be used in a multi threaded manner. +// Specifically, the implementation may call GetCurrentFrame() or +// PutCurrentFrame() from the compositor thread. If so, the caller is +// responsible for making sure Client::DidReceiveFrame() and +// Client::DidUpdateMatrix() are only called from this same thread. class CC_EXPORT VideoFrameProvider { public: + virtual ~VideoFrameProvider() {} + class CC_EXPORT Client { public: - // The provider will call this method to tell the client to stop using it. + // Provider will call this method to tell the client to stop using it. // StopUsingProvider() may be called from any thread. The client should // block until it has PutCurrentFrame() any outstanding frames. virtual void StopUsingProvider() = 0; - // Notifies the client that it should start or stop making regular - // UpdateCurrentFrame() calls to the provider. No further calls to - // UpdateCurrentFrame() should be made once StopRendering() returns. - // - // Callers should use these methods to indicate when it expects and no - // longer expects (respectively) to have new frames for the client. Clients - // may use this information for power conservation. - virtual void StartRendering() = 0; - virtual void StopRendering() = 0; - - // Notifies the client that GetCurrentFrame() will return new data. - // TODO(dalecurtis): Nuke this once VideoFrameProviderClientImpl is using a - // BeginFrameObserver based approach. http://crbug.com/336733 + // Notifies the provider's client that a call to GetCurrentFrame() will + // return new data. virtual void DidReceiveFrame() = 0; - // Notifies the client of a new UV transform matrix to be used. + // Notifies the provider's client of a new UV transform matrix to be used. virtual void DidUpdateMatrix(const float* matrix) = 0; protected: @@ -58,33 +45,18 @@ class CC_EXPORT VideoFrameProvider { // that the provider is not destroyed before this call returns. virtual void SetVideoFrameProviderClient(Client* client) = 0; - // Called by the client on a regular interval. Returns true if a new frame - // will be available via GetCurrentFrame() which should be displayed within - // the presentation interval [|deadline_min|, |deadline_max|]. - // - // Implementations may use this to drive frame acquisition from underlying - // sources, so it must be called by clients before calling GetCurrentFrame(). - virtual bool UpdateCurrentFrame(base::TimeTicks deadline_min, - base::TimeTicks deadline_max) = 0; - - // Returns the current frame, which may have been updated by a recent call to - // UpdateCurrentFrame(). A call to this method does not ensure that the frame - // will be rendered. A subsequent call to PutCurrentFrame() must be made if - // the frame is expected to be rendered. - // - // Clients should call this in response to UpdateCurrentFrame() returning true - // or in response to a DidReceiveFrame() call. - // - // TODO(dalecurtis): Remove text about DidReceiveFrame() once the old path - // has been removed. http://crbug.com/439548 + // This function places a lock on the current frame and returns a pointer to + // it. Calls to this method should always be followed with a call to + // PutCurrentFrame(). + // Only the current provider client should call this function. virtual scoped_refptr GetCurrentFrame() = 0; - // Indicates that the last frame returned via GetCurrentFrame() is expected to - // be rendered. Must only occur after a previous call to GetCurrentFrame(). - virtual void PutCurrentFrame() = 0; - - protected: - virtual ~VideoFrameProvider() {} + // This function releases the lock on the video frame. It should always be + // called after GetCurrentFrame(). Frames passed into this method + // should no longer be referenced after the call is made. Only the current + // provider client should call this function. + virtual void PutCurrentFrame( + const scoped_refptr& frame) = 0; }; } // namespace cc diff --git a/cc/layers/video_frame_provider_client_impl.cc b/cc/layers/video_frame_provider_client_impl.cc index 339d4fc..68e10dc 100644 --- a/cc/layers/video_frame_provider_client_impl.cc +++ b/cc/layers/video_frame_provider_client_impl.cc @@ -78,10 +78,11 @@ VideoFrameProviderClientImpl::AcquireLockAndCurrentFrame() { return provider_->GetCurrentFrame(); } -void VideoFrameProviderClientImpl::PutCurrentFrame() { +void VideoFrameProviderClientImpl::PutCurrentFrame( + const scoped_refptr& frame) { DCHECK(thread_checker_.CalledOnValidThread()); provider_lock_.AssertAcquired(); - provider_->PutCurrentFrame(); + provider_->PutCurrentFrame(frame); } void VideoFrameProviderClientImpl::ReleaseLock() { @@ -103,16 +104,6 @@ void VideoFrameProviderClientImpl::StopUsingProvider() { provider_ = nullptr; } -void VideoFrameProviderClientImpl::StartRendering() { - // TODO(dalecurtis, sunnyps): Hook this method up to control when to start - // observing vsync intervals. http://crbug.com/336733 -} - -void VideoFrameProviderClientImpl::StopRendering() { - // TODO(dalecurtis, sunnyps): Hook this method up to control when to stop - // observing vsync intervals. http://crbug.com/336733 -} - void VideoFrameProviderClientImpl::DidReceiveFrame() { TRACE_EVENT1("cc", "VideoFrameProviderClientImpl::DidReceiveFrame", diff --git a/cc/layers/video_frame_provider_client_impl.h b/cc/layers/video_frame_provider_client_impl.h index ba2eb73..2688ad2 100644 --- a/cc/layers/video_frame_provider_client_impl.h +++ b/cc/layers/video_frame_provider_client_impl.h @@ -37,7 +37,7 @@ class CC_EXPORT VideoFrameProviderClientImpl void Stop(); scoped_refptr AcquireLockAndCurrentFrame(); - void PutCurrentFrame(); + void PutCurrentFrame(const scoped_refptr& frame); void ReleaseLock(); const gfx::Transform& StreamTextureMatrix() const; @@ -46,8 +46,6 @@ class CC_EXPORT VideoFrameProviderClientImpl // Called on the main thread. void StopUsingProvider() override; // Called on the impl thread. - void StartRendering() override; - void StopRendering() override; void DidReceiveFrame() override; void DidUpdateMatrix(const float* matrix) override; diff --git a/cc/layers/video_layer_impl.cc b/cc/layers/video_layer_impl.cc index 0e9add0..3ca0978 100644 --- a/cc/layers/video_layer_impl.cc +++ b/cc/layers/video_layer_impl.cc @@ -380,7 +380,7 @@ void VideoLayerImpl::DidDraw(ResourceProvider* resource_provider) { frame_resources_.clear(); } - provider_client_impl_->PutCurrentFrame(); + provider_client_impl_->PutCurrentFrame(frame_); frame_ = nullptr; provider_client_impl_->ReleaseLock(); diff --git a/cc/resources/display_item.cc b/cc/resources/display_item.cc index 33069a53..52bdec1 100644 --- a/cc/resources/display_item.cc +++ b/cc/resources/display_item.cc @@ -9,4 +9,8 @@ namespace cc { DisplayItem::DisplayItem() { } +void DisplayItem::RasterForTracing(SkCanvas* canvas) const { + Raster(canvas, nullptr); +} + } // namespace cc diff --git a/cc/resources/display_item.h b/cc/resources/display_item.h index d065958..ba4e733 100644 --- a/cc/resources/display_item.h +++ b/cc/resources/display_item.h @@ -21,6 +21,7 @@ class CC_EXPORT DisplayItem { virtual void Raster(SkCanvas* canvas, SkDrawPictureCallback* callback) const = 0; + virtual void RasterForTracing(SkCanvas* canvas) const; virtual bool IsSuitableForGpuRasterization() const = 0; virtual int ApproximateOpCount() const = 0; diff --git a/cc/resources/display_item_list.cc b/cc/resources/display_item_list.cc index 0b2e12e..b541c5f 100644 --- a/cc/resources/display_item_list.cc +++ b/cc/resources/display_item_list.cc @@ -20,36 +20,12 @@ namespace cc { -DisplayItemList::DisplayItemList(gfx::Rect layer_rect, bool use_cached_picture) - : recorder_(new SkPictureRecorder()), - use_cached_picture_(use_cached_picture), - retain_individual_display_items_(!use_cached_picture), - layer_rect_(layer_rect), - is_suitable_for_gpu_rasterization_(true), - approximate_op_count_(0) { - if (use_cached_picture_) { - SkRTreeFactory factory; - recorder_.reset(new SkPictureRecorder()); - canvas_ = skia::SharePtr(recorder_->beginRecording( - layer_rect_.width(), layer_rect_.height(), &factory)); - canvas_->translate(-layer_rect_.x(), -layer_rect_.y()); - canvas_->clipRect(gfx::RectToSkRect(layer_rect_)); - - bool tracing_enabled; - TRACE_EVENT_CATEGORY_GROUP_ENABLED( - TRACE_DISABLED_BY_DEFAULT("cc.debug.picture") "," - TRACE_DISABLED_BY_DEFAULT("devtools.timeline.picture"), - &tracing_enabled); - if (tracing_enabled) - retain_individual_display_items_ = true; - } +DisplayItemList::DisplayItemList() + : is_suitable_for_gpu_rasterization_(true), approximate_op_count_(0) { } -scoped_refptr DisplayItemList::Create( - gfx::Rect layer_rect, - bool use_cached_picture) { - return make_scoped_refptr( - new DisplayItemList(layer_rect, use_cached_picture)); +scoped_refptr DisplayItemList::Create() { + return make_scoped_refptr(new DisplayItemList()); } DisplayItemList::~DisplayItemList() { @@ -58,7 +34,7 @@ DisplayItemList::~DisplayItemList() { void DisplayItemList::Raster(SkCanvas* canvas, SkDrawPictureCallback* callback, float contents_scale) const { - if (!use_cached_picture_) { + if (!picture_) { canvas->save(); canvas->scale(contents_scale, contents_scale); for (size_t i = 0; i < items_.size(); ++i) { @@ -86,25 +62,25 @@ void DisplayItemList::Raster(SkCanvas* canvas, } void DisplayItemList::CreateAndCacheSkPicture() { - // Convert to an SkPicture for faster rasterization. - DCHECK(use_cached_picture_); - picture_ = skia::AdoptRef(recorder_->endRecordingAsPicture()); + // Convert to an SkPicture for faster rasterization. Code is identical to + // that in Picture::Record. + SkRTreeFactory factory; + SkPictureRecorder recorder; + skia::RefPtr canvas; + canvas = skia::SharePtr(recorder.beginRecording( + layer_rect_.width(), layer_rect_.height(), &factory)); + canvas->translate(-layer_rect_.x(), -layer_rect_.y()); + canvas->clipRect(gfx::RectToSkRect(layer_rect_)); + for (size_t i = 0; i < items_.size(); ++i) + items_[i]->Raster(canvas.get(), NULL); + picture_ = skia::AdoptRef(recorder.endRecordingAsPicture()); DCHECK(picture_); - recorder_.reset(); - canvas_.clear(); } void DisplayItemList::AppendItem(scoped_ptr item) { is_suitable_for_gpu_rasterization_ &= item->IsSuitableForGpuRasterization(); approximate_op_count_ += item->ApproximateOpCount(); - - if (use_cached_picture_) { - DCHECK(canvas_); - item->Raster(canvas_.get(), NULL); - } - - if (retain_individual_display_items_) - items_.push_back(item.Pass()); + items_.push_back(item.Pass()); } bool DisplayItemList::IsSuitableForGpuRasterization() const { @@ -150,7 +126,8 @@ DisplayItemList::AsValue() const { recorder.beginRecording(layer_rect_.width(), layer_rect_.height()); canvas->translate(-layer_rect_.x(), -layer_rect_.y()); canvas->clipRect(gfx::RectToSkRect(layer_rect_)); - Raster(canvas, NULL, 1.f); + for (size_t i = 0; i < items_.size(); ++i) + items_[i]->RasterForTracing(canvas); skia::RefPtr picture = skia::AdoptRef(recorder.endRecordingAsPicture()); diff --git a/cc/resources/display_item_list.h b/cc/resources/display_item_list.h index 0a5d468..f490552 100644 --- a/cc/resources/display_item_list.h +++ b/cc/resources/display_item_list.h @@ -18,15 +18,13 @@ class SkCanvas; class SkDrawPictureCallback; -class SkPictureRecorder; namespace cc { class CC_EXPORT DisplayItemList : public base::RefCountedThreadSafe { public: - static scoped_refptr Create(gfx::Rect layer_rect, - bool use_cached_picture); + static scoped_refptr Create(); void Raster(SkCanvas* canvas, SkDrawPictureCallback* callback, @@ -34,6 +32,9 @@ class CC_EXPORT DisplayItemList void AppendItem(scoped_ptr item); + void set_layer_rect(gfx::Rect layer_rect) { layer_rect_ = layer_rect; } + gfx::Rect layer_rect() const { return layer_rect_; } + void CreateAndCacheSkPicture(); bool IsSuitableForGpuRasterization() const; @@ -47,16 +48,11 @@ class CC_EXPORT DisplayItemList void GatherPixelRefs(const gfx::Size& grid_cell_size); private: - DisplayItemList(gfx::Rect layer_rect, bool use_cached_picture); + DisplayItemList(); ~DisplayItemList(); ScopedPtrVector items_; skia::RefPtr picture_; - scoped_ptr recorder_; - skia::RefPtr canvas_; - bool use_cached_picture_; - bool retain_individual_display_items_; - gfx::Rect layer_rect_; bool is_suitable_for_gpu_rasterization_; int approximate_op_count_; diff --git a/cc/resources/display_item_list_unittest.cc b/cc/resources/display_item_list_unittest.cc index 290b25b..b53b84a 100644 --- a/cc/resources/display_item_list_unittest.cc +++ b/cc/resources/display_item_list_unittest.cc @@ -36,9 +36,7 @@ TEST(DisplayItemListTest, SingleDrawingItem) { SkPaint red_paint; red_paint.setColor(SK_ColorRED); unsigned char pixels[4 * 100 * 100] = {0}; - const bool use_cached_picture = true; - scoped_refptr list = - DisplayItemList::Create(layer_rect, use_cached_picture); + scoped_refptr list = DisplayItemList::Create(); gfx::PointF offset(8.f, 9.f); gfx::RectF recording_rect(offset, layer_rect.size()); @@ -49,7 +47,6 @@ TEST(DisplayItemListTest, SingleDrawingItem) { canvas->drawRectCoords(50.f, 50.f, 75.f, 75.f, blue_paint); picture = skia::AdoptRef(recorder.endRecordingAsPicture()); list->AppendItem(DrawingDisplayItem::Create(picture)); - list->CreateAndCacheSkPicture(); DrawDisplayList(pixels, layer_rect, list); SkBitmap expected_bitmap; @@ -79,9 +76,7 @@ TEST(DisplayItemListTest, ClipItem) { SkPaint red_paint; red_paint.setColor(SK_ColorRED); unsigned char pixels[4 * 100 * 100] = {0}; - const bool use_cached_picture = true; - scoped_refptr list = - DisplayItemList::Create(layer_rect, use_cached_picture); + scoped_refptr list = DisplayItemList::Create(); gfx::PointF first_offset(8.f, 9.f); gfx::RectF first_recording_rect(first_offset, layer_rect.size()); @@ -105,7 +100,6 @@ TEST(DisplayItemListTest, ClipItem) { list->AppendItem(DrawingDisplayItem::Create(picture)); list->AppendItem(EndClipDisplayItem::Create()); - list->CreateAndCacheSkPicture(); DrawDisplayList(pixels, layer_rect, list); @@ -137,9 +131,7 @@ TEST(DisplayItemListTest, TransformItem) { SkPaint red_paint; red_paint.setColor(SK_ColorRED); unsigned char pixels[4 * 100 * 100] = {0}; - const bool use_cached_picture = true; - scoped_refptr list = - DisplayItemList::Create(layer_rect, use_cached_picture); + scoped_refptr list = DisplayItemList::Create(); gfx::PointF first_offset(8.f, 9.f); gfx::RectF first_recording_rect(first_offset, layer_rect.size()); @@ -164,7 +156,6 @@ TEST(DisplayItemListTest, TransformItem) { list->AppendItem(DrawingDisplayItem::Create(picture)); list->AppendItem(EndTransformDisplayItem::Create()); - list->CreateAndCacheSkPicture(); DrawDisplayList(pixels, layer_rect, list); @@ -186,13 +177,11 @@ TEST(DisplayItemListTest, TransformItem) { EXPECT_EQ(0, memcmp(pixels, expected_pixels, 4 * 100 * 100)); } -TEST(DisplayItemListTest, FilterItem) { +TEST(DisplayItemList, FilterItem) { gfx::Rect layer_rect(100, 100); FilterOperations filters; unsigned char pixels[4 * 100 * 100] = {0}; - const bool use_cached_picture = true; - scoped_refptr list = - DisplayItemList::Create(layer_rect, use_cached_picture); + scoped_refptr list = DisplayItemList::Create(); SkBitmap source_bitmap; source_bitmap.allocN32Pixels(50, 50); @@ -217,7 +206,6 @@ TEST(DisplayItemListTest, FilterItem) { gfx::RectF filter_bounds(10.f, 10.f, 50.f, 50.f); list->AppendItem(FilterDisplayItem::Create(filters, filter_bounds)); list->AppendItem(EndFilterDisplayItem::Create()); - list->CreateAndCacheSkPicture(); DrawDisplayList(pixels, layer_rect, list); @@ -248,9 +236,8 @@ TEST(DisplayItemListTest, CompactingItems) { gfx::PointF offset(8.f, 9.f); gfx::RectF recording_rect(offset, layer_rect.size()); - bool use_cached_picture = false; - scoped_refptr list_without_caching = - DisplayItemList::Create(layer_rect, use_cached_picture); + scoped_refptr list = DisplayItemList::Create(); + list->set_layer_rect(ToEnclosingRect(recording_rect)); canvas = skia::SharePtr( recorder.beginRecording(gfx::RectFToSkRect(recording_rect))); @@ -258,16 +245,13 @@ TEST(DisplayItemListTest, CompactingItems) { canvas->drawRectCoords(0.f, 0.f, 60.f, 60.f, red_paint); canvas->drawRectCoords(50.f, 50.f, 75.f, 75.f, blue_paint); picture = skia::AdoptRef(recorder.endRecordingAsPicture()); - list_without_caching->AppendItem(DrawingDisplayItem::Create(picture)); - DrawDisplayList(pixels, layer_rect, list_without_caching); + list->AppendItem(DrawingDisplayItem::Create(picture)); + DrawDisplayList(pixels, layer_rect, list); + + list->CreateAndCacheSkPicture(); unsigned char expected_pixels[4 * 100 * 100] = {0}; - use_cached_picture = true; - scoped_refptr list_with_caching = - DisplayItemList::Create(layer_rect, use_cached_picture); - list_with_caching->AppendItem(DrawingDisplayItem::Create(picture)); - list_with_caching->CreateAndCacheSkPicture(); - DrawDisplayList(expected_pixels, layer_rect, list_with_caching); + DrawDisplayList(expected_pixels, layer_rect, list); EXPECT_EQ(0, memcmp(pixels, expected_pixels, 4 * 100 * 100)); } diff --git a/cc/resources/display_list_recording_source.cc b/cc/resources/display_list_recording_source.cc index 701db9b..7ada580 100644 --- a/cc/resources/display_list_recording_source.cc +++ b/cc/resources/display_list_recording_source.cc @@ -119,18 +119,17 @@ bool DisplayListRecordingSource::UpdateAndExpandInvalidation( } } for (int i = 0; i < repeat_count; ++i) { - const bool use_cached_picture = true; - display_list_ = - DisplayItemList::Create(recorded_viewport_, use_cached_picture); - painter->PaintContentsToDisplayList(display_list_.get(), recorded_viewport_, - painting_control); + display_list_ = painter->PaintContentsToDisplayList(recorded_viewport_, + painting_control); } - display_list_->CreateAndCacheSkPicture(); - + display_list_->set_layer_rect(recorded_viewport_); is_suitable_for_gpu_rasterization_ = display_list_->IsSuitableForGpuRasterization(); + DetermineIfSolidColor(); display_list_->EmitTraceSnapshot(); + + display_list_->CreateAndCacheSkPicture(); if (gather_pixel_refs_) display_list_->GatherPixelRefs(grid_cell_size_); diff --git a/cc/resources/drawing_display_item.cc b/cc/resources/drawing_display_item.cc index 91ab3fb..648f9de 100644 --- a/cc/resources/drawing_display_item.cc +++ b/cc/resources/drawing_display_item.cc @@ -34,6 +34,17 @@ void DrawingDisplayItem::Raster(SkCanvas* canvas, canvas->restore(); } +void DrawingDisplayItem::RasterForTracing(SkCanvas* canvas) const { + canvas->save(); + // The picture debugger in about:tracing doesn't drill down into |drawPicture| + // operations. Calling |playback()| rather than |drawPicture()| causes the + // skia operations in |picture_| to appear individually in the picture + // produced for tracing rather than being hidden inside a drawPicture + // operation. + picture_->playback(canvas); + canvas->restore(); +} + bool DrawingDisplayItem::IsSuitableForGpuRasterization() const { return picture_->suitableForGpuRasterization(NULL); } diff --git a/cc/resources/drawing_display_item.h b/cc/resources/drawing_display_item.h index a3eef77..b45a039 100644 --- a/cc/resources/drawing_display_item.h +++ b/cc/resources/drawing_display_item.h @@ -27,6 +27,7 @@ class CC_EXPORT DrawingDisplayItem : public DisplayItem { } void Raster(SkCanvas* canvas, SkDrawPictureCallback* callback) const override; + void RasterForTracing(SkCanvas* canvas) const override; bool IsSuitableForGpuRasterization() const override; int ApproximateOpCount() const override; diff --git a/cc/test/fake_content_layer_client.cc b/cc/test/fake_content_layer_client.cc index c21d87b..296c891 100644 --- a/cc/test/fake_content_layer_client.cc +++ b/cc/test/fake_content_layer_client.cc @@ -71,15 +71,15 @@ void FakeContentLayerClient::PaintContents( } } -void FakeContentLayerClient::PaintContentsToDisplayList( - DisplayItemList* display_list, +scoped_refptr +FakeContentLayerClient::PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting painting_control) { SkPictureRecorder recorder; skia::RefPtr canvas; skia::RefPtr picture; - display_list->AppendItem( - ClipDisplayItem::Create(clip, std::vector())); + scoped_refptr list = DisplayItemList::Create(); + list->AppendItem(ClipDisplayItem::Create(clip, std::vector())); for (RectPaintVector::const_iterator it = draw_rects_.begin(); it != draw_rects_.end(); ++it) { @@ -90,21 +90,21 @@ void FakeContentLayerClient::PaintContentsToDisplayList( canvas->drawRectCoords(draw_rect.x(), draw_rect.y(), draw_rect.width(), draw_rect.height(), paint); picture = skia::AdoptRef(recorder.endRecordingAsPicture()); - display_list->AppendItem(DrawingDisplayItem::Create(picture)); + list->AppendItem(DrawingDisplayItem::Create(picture)); } for (BitmapVector::const_iterator it = draw_bitmaps_.begin(); it != draw_bitmaps_.end(); ++it) { if (!it->transform.IsIdentity()) { - display_list->AppendItem(TransformDisplayItem::Create(it->transform)); + list->AppendItem(TransformDisplayItem::Create(it->transform)); } canvas = skia::SharePtr( recorder.beginRecording(it->bitmap.width(), it->bitmap.height())); canvas->drawBitmap(it->bitmap, it->point.x(), it->point.y(), &it->paint); picture = skia::AdoptRef(recorder.endRecordingAsPicture()); - display_list->AppendItem(DrawingDisplayItem::Create(picture)); + list->AppendItem(DrawingDisplayItem::Create(picture)); if (!it->transform.IsIdentity()) { - display_list->AppendItem(EndTransformDisplayItem::Create()); + list->AppendItem(EndTransformDisplayItem::Create()); } } @@ -118,12 +118,13 @@ void FakeContentLayerClient::PaintContentsToDisplayList( recorder.beginRecording(gfx::RectFToSkRect(draw_rect))); canvas->drawRect(gfx::RectFToSkRect(draw_rect), paint); picture = skia::AdoptRef(recorder.endRecordingAsPicture()); - display_list->AppendItem(DrawingDisplayItem::Create(picture)); + list->AppendItem(DrawingDisplayItem::Create(picture)); draw_rect.Inset(1, 1); } } - display_list->AppendItem(EndClipDisplayItem::Create()); + list->AppendItem(EndClipDisplayItem::Create()); + return list; } bool FakeContentLayerClient::FillsBoundsCompletely() const { return false; } diff --git a/cc/test/fake_content_layer_client.h b/cc/test/fake_content_layer_client.h index d01bbd1..a4ecc59 100644 --- a/cc/test/fake_content_layer_client.h +++ b/cc/test/fake_content_layer_client.h @@ -40,8 +40,7 @@ class FakeContentLayerClient : public ContentLayerClient { void PaintContents(SkCanvas* canvas, const gfx::Rect& rect, PaintingControlSetting painting_control) override; - void PaintContentsToDisplayList( - DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting painting_control) override; bool FillsBoundsCompletely() const override; diff --git a/cc/test/fake_display_list_recording_source.h b/cc/test/fake_display_list_recording_source.h index 544a022..679d5d6 100644 --- a/cc/test/fake_display_list_recording_source.h +++ b/cc/test/fake_display_list_recording_source.h @@ -40,11 +40,9 @@ class FakeDisplayListRecordingSource : public DisplayListRecordingSource { void Rerecord() { ContentLayerClient::PaintingControlSetting painting_control = ContentLayerClient::PAINTING_BEHAVIOR_NORMAL; - bool use_cached_picture = true; - display_list_ = - DisplayItemList::Create(recorded_viewport_, use_cached_picture); - client_.PaintContentsToDisplayList(display_list_.get(), recorded_viewport_, - painting_control); + display_list_ = client_.PaintContentsToDisplayList(recorded_viewport_, + painting_control); + display_list_->set_layer_rect(recorded_viewport_); display_list_->CreateAndCacheSkPicture(); if (gather_pixel_refs_) display_list_->GatherPixelRefs(grid_cell_size_); diff --git a/cc/test/fake_video_frame_provider.cc b/cc/test/fake_video_frame_provider.cc index 3f4b40b..f008d9f 100644 --- a/cc/test/fake_video_frame_provider.cc +++ b/cc/test/fake_video_frame_provider.cc @@ -14,11 +14,6 @@ FakeVideoFrameProvider::~FakeVideoFrameProvider() { client_->StopUsingProvider(); } -bool FakeVideoFrameProvider::UpdateCurrentFrame(base::TimeTicks deadline_min, - base::TimeTicks deadline_max) { - return false; -} - void FakeVideoFrameProvider::SetVideoFrameProviderClient(Client* client) { client_ = client; } diff --git a/cc/test/fake_video_frame_provider.h b/cc/test/fake_video_frame_provider.h index 53903c9..50b6b27 100644 --- a/cc/test/fake_video_frame_provider.h +++ b/cc/test/fake_video_frame_provider.h @@ -17,10 +17,8 @@ class FakeVideoFrameProvider : public VideoFrameProvider { ~FakeVideoFrameProvider() override; void SetVideoFrameProviderClient(Client* client) override; - bool UpdateCurrentFrame(base::TimeTicks deadline_min, - base::TimeTicks deadline_max) override; scoped_refptr GetCurrentFrame() override; - void PutCurrentFrame() override {} + void PutCurrentFrame(const scoped_refptr&) override {} Client* client() { return client_; } diff --git a/cc/test/solid_color_content_layer_client.cc b/cc/test/solid_color_content_layer_client.cc index fe13526..7e97bb8 100644 --- a/cc/test/solid_color_content_layer_client.cc +++ b/cc/test/solid_color_content_layer_client.cc @@ -25,11 +25,12 @@ void SolidColorContentLayerClient::PaintContents( paint); } -void SolidColorContentLayerClient::PaintContentsToDisplayList( - DisplayItemList* display_list, +scoped_refptr +SolidColorContentLayerClient::PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting painting_control) { NOTIMPLEMENTED(); + return DisplayItemList::Create(); } bool SolidColorContentLayerClient::FillsBoundsCompletely() const { diff --git a/cc/test/solid_color_content_layer_client.h b/cc/test/solid_color_content_layer_client.h index 3068f9ae..1ab4c4d 100644 --- a/cc/test/solid_color_content_layer_client.h +++ b/cc/test/solid_color_content_layer_client.h @@ -19,8 +19,7 @@ class SolidColorContentLayerClient : public ContentLayerClient { void PaintContents(SkCanvas* canvas, const gfx::Rect& rect, PaintingControlSetting painting_control) override; - void PaintContentsToDisplayList( - DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting painting_control) override; bool FillsBoundsCompletely() const override; diff --git a/cc/trees/layer_tree_host_common_unittest.cc b/cc/trees/layer_tree_host_common_unittest.cc index 0046b1a..439d920 100644 --- a/cc/trees/layer_tree_host_common_unittest.cc +++ b/cc/trees/layer_tree_host_common_unittest.cc @@ -60,11 +60,11 @@ class MockContentLayerClient : public ContentLayerClient { void PaintContents(SkCanvas* canvas, const gfx::Rect& clip, PaintingControlSetting picture_control) override {} - void PaintContentsToDisplayList( - DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting picture_control) override { NOTIMPLEMENTED(); + return DisplayItemList::Create(); } bool FillsBoundsCompletely() const override { return false; } }; diff --git a/cc/trees/layer_tree_host_pixeltest_masks.cc b/cc/trees/layer_tree_host_pixeltest_masks.cc index 0cb6aea..97765bd 100644 --- a/cc/trees/layer_tree_host_pixeltest_masks.cc +++ b/cc/trees/layer_tree_host_pixeltest_masks.cc @@ -46,11 +46,11 @@ class MaskContentLayerClient : public ContentLayerClient { } } - void PaintContentsToDisplayList( - DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting picture_control) override { NOTIMPLEMENTED(); + return DisplayItemList::Create(); } private: @@ -303,11 +303,11 @@ class CheckerContentLayerClient : public ContentLayerClient { } } } - void PaintContentsToDisplayList( - DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting picture_control) override { NOTIMPLEMENTED(); + return DisplayItemList::Create(); } private: @@ -334,11 +334,11 @@ class CircleContentLayerClient : public ContentLayerClient { bounds_.width() / 4, paint); } - void PaintContentsToDisplayList( - DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting picture_control) override { NOTIMPLEMENTED(); + return DisplayItemList::Create(); } private: diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc index ff8c5e8..db7bb9c 100644 --- a/cc/trees/layer_tree_host_unittest.cc +++ b/cc/trees/layer_tree_host_unittest.cc @@ -1326,11 +1326,11 @@ class TestOpacityChangeLayerDelegate : public ContentLayerClient { if (test_layer_) test_layer_->SetOpacity(0.f); } - void PaintContentsToDisplayList( - DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting picture_control) override { NOTIMPLEMENTED(); + return DisplayItemList::Create(); } bool FillsBoundsCompletely() const override { return false; } @@ -2791,11 +2791,11 @@ class LayerTreeHostTestChangeLayerPropertiesInPaintContents layer_->SetBounds(gfx::Size(2, 2)); } - void PaintContentsToDisplayList( - DisplayItemList* display_list, + scoped_refptr PaintContentsToDisplayList( const gfx::Rect& clip, PaintingControlSetting picture_control) override { NOTIMPLEMENTED(); + return DisplayItemList::Create(); } bool FillsBoundsCompletely() const override { return false; } diff --git a/chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/FakeServerHelper.java b/chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/FakeServerHelper.java index b25eca1..d2a6d15 100644 --- a/chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/FakeServerHelper.java +++ b/chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/FakeServerHelper.java @@ -71,8 +71,11 @@ public class FakeServerHelper { * Deletes the existing FakeServer. */ public static void deleteFakeServer() { - checkFakeServerInitialized( - "useFakeServer must be called before calling deleteFakeServer."); + if (sNativeFakeServer == 0L) { + throw new IllegalStateException( + "useFakeServer must be called before calling deleteFakeServer."); + } + ThreadUtils.runOnUiThreadBlockingNoException(new Callable() { @Override public Void call() { @@ -124,8 +127,10 @@ public class FakeServerHelper { * @return whether the number of specified entities exist */ public boolean verifyEntityCountByTypeAndName(int count, ModelType modelType, String name) { - checkFakeServerInitialized( + if (sNativeFakeServer == 0L) { + throw new IllegalStateException( "useFakeServer must be called before data verification."); + } return nativeVerifyEntityCountByTypeAndName(mNativeFakeServerHelperAndroid, sNativeFakeServer, count, modelType.toString(), name); } @@ -139,44 +144,16 @@ public class FakeServerHelper { * @param entitySpecifics the EntitySpecifics proto that represents the entity to inject */ public void injectUniqueClientEntity(String name, EntitySpecifics entitySpecifics) { - checkFakeServerInitialized("useFakeServer must be called before data injection."); + if (sNativeFakeServer == 0L) { + throw new IllegalStateException( + "useFakeServer must be called before data injection."); + } // The protocol buffer is serialized as a byte array because it can be easily deserialized // from this format in native code. nativeInjectUniqueClientEntity(mNativeFakeServerHelperAndroid, sNativeFakeServer, name, MessageNano.toByteArray(entitySpecifics)); } - /** - * Injects a bookmark into the fake Sync server. - * - * @param title the title of the bookmark to inject - * @param url the URL of the bookmark to inject. This String will be passed to the native GURL - * class, so it must be a valid URL under its definition. - * @param parentId the ID of the desired parent bookmark folder - */ - public void injectBookmarkEntity(String title, String url, String parentId) { - checkFakeServerInitialized("useFakeServer must be called before data injection."); - nativeInjectBookmarkEntity(mNativeFakeServerHelperAndroid, sNativeFakeServer, title, url, - parentId); - } - - /** - * Returns the ID of the Bookmark Bar. This value is to be used in conjunction with - * injectBookmarkEntity. - * - * @return the opaque ID of the bookmark bar entity stored in the server - */ - public String getBookmarkBarFolderId() { - checkFakeServerInitialized("useFakeServer must be called before access"); - return nativeGetBookmarkBarFolderId(mNativeFakeServerHelperAndroid, sNativeFakeServer); - } - - private static void checkFakeServerInitialized(String failureMessage) { - if (sNativeFakeServer == 0L) { - throw new IllegalStateException(failureMessage); - } - } - // Native methods. private native long nativeInit(); private native long nativeCreateFakeServer(long nativeFakeServerHelperAndroid); @@ -190,9 +167,4 @@ public class FakeServerHelper { private native void nativeInjectUniqueClientEntity( long nativeFakeServerHelperAndroid, long nativeFakeServer, String name, byte[] serializedEntitySpecifics); - private native void nativeInjectBookmarkEntity( - long nativeFakeServerHelperAndroid, long nativeFakeServer, String title, String url, - String parentId); - private native String nativeGetBookmarkBarFolderId( - long nativeFakeServerHelperAndroid, long nativeFakeServer); } diff --git a/chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java b/chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java index 27226bd..730500c 100644 --- a/chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java +++ b/chrome/android/sync_shell/javatests/src/chromium/chrome/browser/sync/SyncTest.java @@ -291,23 +291,6 @@ public class SyncTest extends ChromeShellTestBase { // injected. This data should be retrieved from the Sync node browser data. } - @LargeTest - @Feature({"Sync"}) - public void testDownloadBookmark() throws InterruptedException { - setupTestAccountAndSignInToSync(FOREIGN_SESSION_TEST_MACHINE_ID); - // 3 bookmark folders exist by default: Bookmarks Bar, Other Bookmarks, Mobile Bookmarks. - assertLocalEntityCount("Bookmarks", 3); - - mFakeServerHelper.injectBookmarkEntity( - "Title", "http://chromium.org", mFakeServerHelper.getBookmarkBarFolderId()); - - SyncTestUtil.triggerSyncAndWaitForCompletion(mContext); - assertLocalEntityCount("Bookmarks", 4); - - // TODO(pvalenzuela): Also verify that the downloaded bookmark matches the one that was - // injected. This data should be retrieved from the Sync node browser data. - } - private void setupTestAccountAndSignInToSync( final String syncClientIdentifier) throws InterruptedException { diff --git a/chrome/browser/chromeos/login/helper.cc b/chrome/browser/chromeos/login/helper.cc index f949875..afe1bf4 100644 --- a/chrome/browser/chromeos/login/helper.cc +++ b/chrome/browser/chromeos/login/helper.cc @@ -153,20 +153,8 @@ content::StoragePartition* GetSigninPartition() { } net::URLRequestContextGetter* GetSigninContext() { - if (StartupUtils::IsWebviewSigninEnabled()) { - content::StoragePartition* signin_partition = GetSigninPartition(); - - // Special case for unit tests. There's no LoginDisplayHost thus no - // webview instance. TODO(nkostylev): Investigate if there's a better - // place to address this like dependency injection. http://crbug.com/477402 - if (!signin_partition && !LoginDisplayHostImpl::default_host()) - return ProfileHelper::GetSigninProfile()->GetRequestContext(); - - if (!signin_partition) - return nullptr; - - return signin_partition->GetURLRequestContext(); - } + if (StartupUtils::IsWebviewSigninEnabled()) + return GetSigninPartition()->GetURLRequestContext(); return ProfileHelper::GetSigninProfile()->GetRequestContext(); } diff --git a/chrome/browser/chromeos/login/hid_detection_browsertest.cc b/chrome/browser/chromeos/login/hid_detection_browsertest.cc index d4e31b2..ebb23d2 100644 --- a/chrome/browser/chromeos/login/hid_detection_browsertest.cc +++ b/chrome/browser/chromeos/login/hid_detection_browsertest.cc @@ -43,15 +43,11 @@ void SetUpBluetoothMock( namespace chromeos { -// Boolean parameter is used to run this test for webview (true) and for -// iframe (false) GAIA sign in. -class HidDetectionTest : public OobeBaseTest, - public testing::WithParamInterface { +class HidDetectionTest : public OobeBaseTest { public: typedef device::InputServiceLinux::InputDeviceInfo InputDeviceInfo; HidDetectionTest() : weak_ptr_factory_(this) { - set_use_webview(GetParam()); InputServiceProxy::SetThreadIdForTesting(content::BrowserThread::UI); HidDetectionTest::InitInputService(); } @@ -116,17 +112,13 @@ class HidDetectionSkipTest : public HidDetectionTest { } }; -IN_PROC_BROWSER_TEST_P(HidDetectionTest, NoDevicesConnected) { +IN_PROC_BROWSER_TEST_F(HidDetectionTest, NoDevicesConnected) { OobeScreenWaiter(OobeDisplay::SCREEN_OOBE_HID_DETECTION).Wait(); } -IN_PROC_BROWSER_TEST_P(HidDetectionSkipTest, BothDevicesPreConnected) { +IN_PROC_BROWSER_TEST_F(HidDetectionSkipTest, BothDevicesPreConnected) { OobeScreenWaiter(OobeDisplay::SCREEN_OOBE_NETWORK).Wait(); -} -INSTANTIATE_TEST_CASE_P(HidDetectionSuite, HidDetectionTest, testing::Bool()); -INSTANTIATE_TEST_CASE_P(HidDetectionSkipSuite, - HidDetectionSkipTest, - testing::Bool()); +} } // namespace chromeos diff --git a/chrome/browser/chromeos/login/login_browsertest.cc b/chrome/browser/chromeos/login/login_browsertest.cc index 5d1cffa..85880b1 100644 --- a/chrome/browser/chromeos/login/login_browsertest.cc +++ b/chrome/browser/chromeos/login/login_browsertest.cc @@ -5,8 +5,6 @@ #include "ash/shell.h" #include "ash/system/tray/system_tray.h" #include "base/command_line.h" -#include "base/json/json_file_value_serializer.h" -#include "base/path_service.h" #include "base/strings/string_util.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chromeos/login/login_manager_test.h" @@ -19,9 +17,7 @@ #include "chrome/browser/profiles/profiles_state.h" #include "chrome/browser/ui/browser.h" #include "chrome/common/chrome_constants.h" -#include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" -#include "chrome/common/pref_names.h" #include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/interactive_test_utils.h" #include "chrome/test/base/tracing.h" @@ -147,33 +143,6 @@ class LoginTest : public chromeos::LoginManagerTest { } }; -class LoginOfflineTest : public LoginTest { - public: - LoginOfflineTest() {} - ~LoginOfflineTest() override {} - - bool SetUpUserDataDirectory() override { - base::FilePath user_data_dir; - CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)); - base::FilePath local_state_path = - user_data_dir.Append(chrome::kLocalStateFilename); - base::DictionaryValue local_state_dict; - - // Set webview disabled flag only when local state file does not exist. - // Otherwise, we break PRE tests that leave state in it. - if (!base::PathExists(local_state_path)) { - // TODO(rsorokin): Fix offline test for webview signin. - // http://crbug.com/475569 - local_state_dict.SetBoolean(prefs::kWebviewSigninDisabled, true); - - CHECK(JSONFileValueSerializer(local_state_path) - .Serialize(local_state_dict)); - } - - return LoginTest::SetUpUserDataDirectory(); - } -}; - // Used to make sure that the system tray is visible and within the screen // bounds after login. void TestSystemTrayIsVisible() { @@ -246,14 +215,14 @@ IN_PROC_BROWSER_TEST_F(LoginSigninTest, WebUIVisible) { ASSERT_TRUE(tracing::EndTracing(&json_events)); } -IN_PROC_BROWSER_TEST_F(LoginOfflineTest, PRE_GaiaAuthOffline) { +IN_PROC_BROWSER_TEST_F(LoginTest, PRE_GaiaAuthOffline) { RegisterUser(kTestUser); chromeos::StartupUtils::MarkOobeCompleted(); chromeos::CrosSettings::Get()->SetBoolean( chromeos::kAccountsPrefShowUserNamesOnSignIn, false); } -IN_PROC_BROWSER_TEST_F(LoginOfflineTest, GaiaAuthOffline) { +IN_PROC_BROWSER_TEST_F(LoginTest, GaiaAuthOffline) { bool show_user; ASSERT_TRUE(chromeos::CrosSettings::Get()->GetBoolean( chromeos::kAccountsPrefShowUserNamesOnSignIn, &show_user)); diff --git a/chrome/browser/chromeos/login/login_manager_test.cc b/chrome/browser/chromeos/login/login_manager_test.cc index bd44832..45a6101 100644 --- a/chrome/browser/chromeos/login/login_manager_test.cc +++ b/chrome/browser/chromeos/login/login_manager_test.cc @@ -5,20 +5,14 @@ #include "chrome/browser/chromeos/login/login_manager_test.h" #include "base/command_line.h" -#include "base/json/json_file_value_serializer.h" -#include "base/path_service.h" #include "base/prefs/scoped_user_pref_update.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chromeos/login/existing_user_controller.h" #include "chrome/browser/chromeos/login/session/user_session_manager.h" #include "chrome/browser/chromeos/login/session/user_session_manager_test_api.h" -#include "chrome/browser/chromeos/login/startup_utils.h" #include "chrome/browser/chromeos/login/ui/login_display_host_impl.h" #include "chrome/browser/chromeos/login/ui/webui_login_view.h" -#include "chrome/common/chrome_constants.h" -#include "chrome/common/chrome_paths.h" -#include "chrome/common/pref_names.h" #include "chromeos/chromeos_switches.h" #include "chromeos/login/auth/key.h" #include "chromeos/login/auth/user_context.h" @@ -33,8 +27,7 @@ namespace chromeos { LoginManagerTest::LoginManagerTest(bool should_launch_browser) - : use_webview_(false), - should_launch_browser_(should_launch_browser), + : should_launch_browser_(should_launch_browser), web_contents_(NULL) { set_exit_when_last_browser_closes(false); } @@ -68,29 +61,6 @@ void LoginManagerTest::SetUpOnMainThread() { should_launch_browser_); } -bool LoginManagerTest::SetUpUserDataDirectory() { - base::FilePath user_data_dir; - CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)); - base::FilePath local_state_path = - user_data_dir.Append(chrome::kLocalStateFilename); - - if (!use_webview()) { - // Set webview disabled flag only when local state file does not exist. - // Otherwise, we break PRE tests that leave state in it. - if (!base::PathExists(local_state_path)) { - base::DictionaryValue local_state_dict; - - // TODO(nkostylev): Fix tests that fail with webview signin. - local_state_dict.SetBoolean(prefs::kWebviewSigninDisabled, true); - - CHECK(JSONFileValueSerializer(local_state_path) - .Serialize(local_state_dict)); - } - } - - return MixinBasedBrowserTest::SetUpUserDataDirectory(); -} - void LoginManagerTest::RegisterUser(const std::string& user_id) { ListPrefUpdate users_pref(g_browser_process->local_state(), "LoggedInUsers"); users_pref->AppendIfNotPresent(new base::StringValue(user_id)); diff --git a/chrome/browser/chromeos/login/login_manager_test.h b/chrome/browser/chromeos/login/login_manager_test.h index ff4a6dd..e2b6921 100644 --- a/chrome/browser/chromeos/login/login_manager_test.h +++ b/chrome/browser/chromeos/login/login_manager_test.h @@ -33,7 +33,6 @@ class LoginManagerTest : public MixinBasedBrowserTest { void SetUpCommandLine(base::CommandLine* command_line) override; void SetUpInProcessBrowserTestFixture() override; void SetUpOnMainThread() override; - bool SetUpUserDataDirectory() override; // Registers the user with the given |user_id| on the device. // This method should be called in PRE_* test. @@ -67,12 +66,6 @@ class LoginManagerTest : public MixinBasedBrowserTest { test::JSChecker& js_checker() { return js_checker_; } - protected: - bool use_webview() { return use_webview_; } - void set_use_webview(bool use_webview) { use_webview_ = use_webview; } - - bool use_webview_; - private: void InitializeWebContents(); diff --git a/chrome/browser/chromeos/login/oobe_browsertest.cc b/chrome/browser/chromeos/login/oobe_browsertest.cc index 2f13bd1..da7e596 100644 --- a/chrome/browser/chromeos/login/oobe_browsertest.cc +++ b/chrome/browser/chromeos/login/oobe_browsertest.cc @@ -5,7 +5,6 @@ #include "base/command_line.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chromeos/login/existing_user_controller.h" -#include "chrome/browser/chromeos/login/test/oobe_base_test.h" #include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h" #include "chrome/browser/chromeos/login/ui/login_display_host_impl.h" #include "chrome/browser/chromeos/login/ui/webui_login_display.h" @@ -15,8 +14,8 @@ #include "chrome/common/chrome_switches.h" #include "chrome/test/base/in_process_browser_test.h" #include "chromeos/chromeos_switches.h" -#include "content/public/test/browser_test_utils.h" #include "content/public/test/test_utils.h" +#include "google_apis/gaia/fake_gaia.h" #include "google_apis/gaia/gaia_switches.h" #include "net/test/embedded_test_server/embedded_test_server.h" #include "net/test/embedded_test_server/http_response.h" @@ -28,17 +27,28 @@ using namespace net::test_server; namespace chromeos { -// Boolean parameter is used to run this test for webview (true) and for -// iframe (false) GAIA sign in. -class OobeTest : public OobeBaseTest, public testing::WithParamInterface { +class OobeTest : public InProcessBrowserTest { public: - OobeTest() { set_use_webview(GetParam()); } + OobeTest() {} ~OobeTest() override {} void SetUpCommandLine(base::CommandLine* command_line) override { - command_line->AppendSwitch(switches::kOobeSkipPostLogin); + command_line->AppendSwitch(chromeos::switches::kLoginManager); + command_line->AppendSwitch(chromeos::switches::kForceLoginManagerInTests); + command_line->AppendSwitchASCII(chromeos::switches::kLoginProfile, "user"); + command_line->AppendSwitchASCII( + ::switches::kAuthExtensionPath, "gaia_auth"); + fake_gaia_.Initialize(); + } + + void SetUpOnMainThread() override { + CHECK(embedded_test_server()->InitializeAndWaitUntilReady()); + embedded_test_server()->RegisterRequestHandler( + base::Bind(&FakeGaia::HandleRequest, base::Unretained(&fake_gaia_))); + LOG(INFO) << "Set up http server at " << embedded_test_server()->base_url(); - OobeBaseTest::SetUpCommandLine(command_line); + base::CommandLine::ForCurrentProcess()->AppendSwitchASCII( + ::switches::kGaiaUrl, embedded_test_server()->base_url().spec()); } void TearDownOnMainThread() override { @@ -48,8 +58,6 @@ class OobeTest : public OobeBaseTest, public testing::WithParamInterface { base::Bind(&chrome::AttemptExit)); content::RunMessageLoop(); } - - OobeBaseTest::TearDownOnMainThread(); } chromeos::WebUILoginDisplay* GetLoginDisplay() { @@ -67,24 +75,40 @@ class OobeTest : public OobeBaseTest, public testing::WithParamInterface { } private: + FakeGaia fake_gaia_; DISALLOW_COPY_AND_ASSIGN(OobeTest); }; -IN_PROC_BROWSER_TEST_P(OobeTest, NewUser) { - WaitForGaiaPageLoad(); +IN_PROC_BROWSER_TEST_F(OobeTest, NewUser) { + chromeos::WizardController::SkipPostLoginScreensForTesting(); + chromeos::WizardController* wizard_controller = + chromeos::WizardController::default_controller(); + CHECK(wizard_controller); + wizard_controller->SkipToLoginForTesting(LoginScreenContext()); - content::WindowedNotificationObserver session_start_waiter( - chrome::NOTIFICATION_SESSION_STARTED, - content::NotificationService::AllSources()); + content::WindowedNotificationObserver( + chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE, + content::NotificationService::AllSources()).Wait(); - GetLoginDisplay()->ShowSigninScreenForCreds(OobeBaseTest::kFakeUserEmail, - OobeBaseTest::kFakeUserPassword); + // TODO(glotov): mock GAIA server (test_server()) should support + // username/password configuration. + GetLoginDisplay()->ShowSigninScreenForCreds("username", "password"); - session_start_waiter.Wait(); + content::WindowedNotificationObserver( + chrome::NOTIFICATION_SESSION_STARTED, + content::NotificationService::AllSources()).Wait(); } -IN_PROC_BROWSER_TEST_P(OobeTest, Accelerator) { - WaitForGaiaPageLoad(); +IN_PROC_BROWSER_TEST_F(OobeTest, Accelerator) { + chromeos::WizardController::SkipPostLoginScreensForTesting(); + chromeos::WizardController* wizard_controller = + chromeos::WizardController::default_controller(); + CHECK(wizard_controller); + wizard_controller->SkipToLoginForTesting(LoginScreenContext()); + + content::WindowedNotificationObserver( + chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE, + content::NotificationService::AllSources()).Wait(); gfx::NativeWindow login_window = GetLoginWindowWidget()->GetNativeWindow(); @@ -97,6 +121,4 @@ IN_PROC_BROWSER_TEST_P(OobeTest, Accelerator) { OobeScreenWaiter(OobeDisplay::SCREEN_OOBE_ENROLLMENT).Wait(); } -INSTANTIATE_TEST_CASE_P(OobeSuite, OobeTest, testing::Bool()); - } // namespace chromeos diff --git a/chrome/browser/chromeos/login/proxy_auth_dialog_browsertest.cc b/chrome/browser/chromeos/login/proxy_auth_dialog_browsertest.cc index 3eef72f..eca6b5c 100644 --- a/chrome/browser/chromeos/login/proxy_auth_dialog_browsertest.cc +++ b/chrome/browser/chromeos/login/proxy_auth_dialog_browsertest.cc @@ -57,21 +57,13 @@ class ProxyAuthDialogWaiter : public content::WindowedNotificationObserver { } // namespace -// Boolean parameter is used to run this test for webview (true) and for -// iframe (false) GAIA sign in. -class ProxyAuthOnUserBoardScreenTest - : public LoginManagerTest, - public testing::WithParamInterface { +class ProxyAuthOnUserBoardScreenTest : public LoginManagerTest { public: ProxyAuthOnUserBoardScreenTest() : LoginManagerTest(true /* should_launch_browser */), proxy_server_(net::SpawnedTestServer::TYPE_BASIC_AUTH_PROXY, net::SpawnedTestServer::kLocalhost, - base::FilePath()) { - // TODO(paulmeyer): Re-enable webview version of this test - // (uncomment this line) once http://crbug.com/452452 is fixed. - // set_use_webview(GetParam()); - } + base::FilePath()) {} ~ProxyAuthOnUserBoardScreenTest() override {} @@ -92,13 +84,13 @@ class ProxyAuthOnUserBoardScreenTest DISALLOW_COPY_AND_ASSIGN(ProxyAuthOnUserBoardScreenTest); }; -IN_PROC_BROWSER_TEST_P(ProxyAuthOnUserBoardScreenTest, +IN_PROC_BROWSER_TEST_F(ProxyAuthOnUserBoardScreenTest, PRE_ProxyAuthDialogOnUserBoardScreen) { RegisterUser("test-user@gmail.com"); StartupUtils::MarkOobeCompleted(); } -IN_PROC_BROWSER_TEST_P(ProxyAuthOnUserBoardScreenTest, +IN_PROC_BROWSER_TEST_F(ProxyAuthOnUserBoardScreenTest, ProxyAuthDialogOnUserBoardScreen) { LoginDisplayHost* login_display_host = LoginDisplayHostImpl::default_host(); WebUILoginView* web_ui_login_view = login_display_host->GetWebUILoginView(); @@ -128,8 +120,4 @@ IN_PROC_BROWSER_TEST_P(ProxyAuthOnUserBoardScreenTest, } } -INSTANTIATE_TEST_CASE_P(ProxyAuthOnUserBoardScreenTestSuite, - ProxyAuthOnUserBoardScreenTest, - testing::Bool()); - } // namespace chromeos diff --git a/chrome/browser/chromeos/login/saml/saml_browsertest.cc b/chrome/browser/chromeos/login/saml/saml_browsertest.cc index ce4d3c0..0a11e4c 100644 --- a/chrome/browser/chromeos/login/saml/saml_browsertest.cc +++ b/chrome/browser/chromeos/login/saml/saml_browsertest.cc @@ -170,7 +170,7 @@ void FakeSamlIdp::SetUp(const std::string& base_path, const GURL& gaia_url) { ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir)); html_template_dir_ = test_data_dir.Append("login"); - login_path_ = base_path; + login_path_= base_path; login_auth_path_ = base_path + "Auth"; gaia_assertion_url_ = gaia_url.Resolve("/SSO"); } @@ -259,14 +259,9 @@ scoped_ptr FakeSamlIdp::BuildHTMLResponse( } // namespace -// Boolean parameter is used to run this test for webview (true) and for -// iframe (false) GAIA sign in. class SamlTest : public OobeBaseTest, public testing::WithParamInterface { public: - SamlTest() : saml_load_injected_(false) { - set_use_webview(GetParam()); - set_initialize_fake_merge_session(false); - } + SamlTest() : saml_load_injected_(false) { use_webview_ = GetParam(); } ~SamlTest() override {} void SetUpCommandLine(base::CommandLine* command_line) override { @@ -370,13 +365,13 @@ IN_PROC_BROWSER_TEST_P(SamlTest, SamlUI) { // Saml flow UI expectations. JsExpect("$('gaia-signin').classList.contains('full-width')"); - if (!use_webview()) { + if (!use_webview_) { JsExpect("!$('cancel-add-user-button').hidden"); } // Click on 'cancel'. content::DOMMessageQueue message_queue; // Observe before 'cancel'. - if (use_webview()) { + if (use_webview_) { ASSERT_TRUE(content::ExecuteScript( GetLoginUI()->GetWebContents(), "$('close-button-item').click();")); @@ -402,7 +397,7 @@ IN_PROC_BROWSER_TEST_P(SamlTest, CredentialPassingAPI) { // webview.executeScript and there is no way to control the injection time. // As a result, this test is flaky and fails about 20% of the time. // TODO(xiyuan): Re-enable when webview.addContentScript API is ready. - if (use_webview()) + if (use_webview_) return; fake_saml_idp()->SetLoginHTMLTemplate("saml_api_login.html"); diff --git a/chrome/browser/chromeos/login/session/user_session_manager.cc b/chrome/browser/chromeos/login/session/user_session_manager.cc index 50f75fb..31395c2 100644 --- a/chrome/browser/chromeos/login/session/user_session_manager.cc +++ b/chrome/browser/chromeos/login/session/user_session_manager.cc @@ -997,29 +997,14 @@ void UserSessionManager::UserProfileInitialized(Profile* profile, // empty if |transfer_saml_auth_cookies_on_subsequent_login| is true. const bool transfer_auth_cookies_and_channel_ids_on_first_login = has_auth_cookies_; - - net::URLRequestContextGetter* auth_request_context = - GetAuthRequestContext(); - - // Authentication request context may be missing especially if user didn't - // sign in using GAIA (webview) and webview didn't yet initialize. - if (auth_request_context) { - ProfileAuthData::Transfer( - auth_request_context, profile->GetRequestContext(), - transfer_auth_cookies_and_channel_ids_on_first_login, - transfer_saml_auth_cookies_on_subsequent_login, - base::Bind( - &UserSessionManager::CompleteProfileCreateAfterAuthTransfer, - AsWeakPtr(), profile)); - } else { - // We need to post task so that OnProfileCreated() caller sends out - // NOTIFICATION_PROFILE_CREATED which marks user profile as initialized. - base::MessageLoopProxy::current()->PostTask( - FROM_HERE, - base::Bind( - &UserSessionManager::CompleteProfileCreateAfterAuthTransfer, - AsWeakPtr(), profile)); - } + ProfileAuthData::Transfer( + GetAuthRequestContext(), + profile->GetRequestContext(), + transfer_auth_cookies_and_channel_ids_on_first_login, + transfer_saml_auth_cookies_on_subsequent_login, + base::Bind(&UserSessionManager::CompleteProfileCreateAfterAuthTransfer, + AsWeakPtr(), + profile)); return; } @@ -1246,18 +1231,9 @@ void UserSessionManager::RestoreAuthSessionImpl( OAuth2LoginManagerFactory::GetInstance()->GetForProfile(profile); login_manager->AddObserver(this); - net::URLRequestContextGetter* auth_request_context = GetAuthRequestContext(); - - // Authentication request context may not be available if user was not - // signing in with GAIA webview (i.e. webview instance hasn't been - // initialized at all). Use fallback request context. - if (!auth_request_context) { - auth_request_context = - authenticator_->authentication_context()->GetRequestContext(); - } - login_manager->RestoreSession(auth_request_context, session_restore_strategy_, - user_context_.GetRefreshToken(), - user_context_.GetAuthCode()); + login_manager->RestoreSession( + GetAuthRequestContext(), session_restore_strategy_, + user_context_.GetRefreshToken(), user_context_.GetAuthCode()); } void UserSessionManager::InitRlzImpl(Profile* profile, bool disabled) { @@ -1446,15 +1422,13 @@ void UserSessionManager::UpdateEasyUnlockKeys(const UserContext& user_context) { net::URLRequestContextGetter* UserSessionManager::GetAuthRequestContext() const { - net::URLRequestContextGetter* auth_request_context = nullptr; + net::URLRequestContextGetter* auth_request_context = NULL; if (StartupUtils::IsWebviewSigninEnabled()) { // Webview uses different partition storage than iframe. We need to get // cookies from the right storage for url request to get auth token into // session. - content::StoragePartition* signin_partition = login::GetSigninPartition(); - if (signin_partition) - auth_request_context = signin_partition->GetURLRequestContext(); + auth_request_context = login::GetSigninPartition()->GetURLRequestContext(); } else if (authenticator_.get() && authenticator_->authentication_context()) { auth_request_context = authenticator_->authentication_context()->GetRequestContext(); diff --git a/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc b/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc index c09e3a7..48bdb7b 100644 --- a/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc +++ b/chrome/browser/chromeos/login/signin/oauth2_browsertest.cc @@ -13,7 +13,6 @@ #include "chrome/browser/chromeos/login/signin/oauth2_login_manager.h" #include "chrome/browser/chromeos/login/signin/oauth2_login_manager_factory.h" #include "chrome/browser/chromeos/login/signin_specifics.h" -#include "chrome/browser/chromeos/login/startup_utils.h" #include "chrome/browser/chromeos/login/test/oobe_base_test.h" #include "chrome/browser/chromeos/login/wizard_controller.h" #include "chrome/browser/profiles/profile_manager.h" @@ -131,12 +130,9 @@ class OAuth2LoginManagerStateWaiter : public OAuth2LoginManager::Observer { } // namespace -// Boolean parameter is used to run this test for webview (true) and for -// iframe (false) GAIA sign in. -class OAuth2Test : public OobeBaseTest, - public testing::WithParamInterface { +class OAuth2Test : public OobeBaseTest { protected: - OAuth2Test() { set_use_webview(GetParam()); } + OAuth2Test() {} void SetUpCommandLine(base::CommandLine* command_line) override { OobeBaseTest::SetUpCommandLine(command_line); @@ -331,17 +327,23 @@ class OAuth2Test : public OobeBaseTest, void StartNewUserSession(bool wait_for_merge) { SetupGaiaServerForNewAccount(); SimulateNetworkOnline(); - WaitForGaiaPageLoad(); + chromeos::WizardController::SkipPostLoginScreensForTesting(); + chromeos::WizardController* wizard_controller = + chromeos::WizardController::default_controller(); + wizard_controller->SkipToLoginForTesting(LoginScreenContext()); - content::WindowedNotificationObserver session_start_waiter( - chrome::NOTIFICATION_SESSION_STARTED, - content::NotificationService::AllSources()); + content::WindowedNotificationObserver( + chrome::NOTIFICATION_LOGIN_OR_LOCK_WEBUI_VISIBLE, + content::NotificationService::AllSources()).Wait(); // Use capitalized and dotted user name on purpose to make sure // our email normalization kicks in. GetLoginDisplay()->ShowSigninScreenForCreds(kTestRawAccountId, kTestAccountPassword); - session_start_waiter.Wait(); + + content::WindowedNotificationObserver( + chrome::NOTIFICATION_SESSION_STARTED, + content::NotificationService::AllSources()).Wait(); if (wait_for_merge) { // Wait for the session merge to finish. @@ -412,7 +414,7 @@ class CookieReader : public base::RefCountedThreadSafe { }; // PRE_MergeSession is testing merge session for a new profile. -IN_PROC_BROWSER_TEST_P(OAuth2Test, PRE_PRE_PRE_MergeSession) { +IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_PRE_PRE_MergeSession) { StartNewUserSession(true); // Check for existance of refresh token. ProfileOAuth2TokenService* token_service = @@ -422,6 +424,7 @@ IN_PROC_BROWSER_TEST_P(OAuth2Test, PRE_PRE_PRE_MergeSession) { EXPECT_EQ(GetOAuthStatusFromLocalState(kTestAccountId), user_manager::User::OAUTH2_TOKEN_STATUS_VALID); + scoped_refptr cookie_reader(new CookieReader()); cookie_reader->ReadCookies(profile()); EXPECT_EQ(cookie_reader->GetCookieValue("SID"), kTestSessionSIDCookie); @@ -432,7 +435,7 @@ IN_PROC_BROWSER_TEST_P(OAuth2Test, PRE_PRE_PRE_MergeSession) { // that was generated in PRE_PRE_PRE_MergeSession test. In this test, we // are not running /MergeSession process since the /ListAccounts call confirms // that the session is not stale. -IN_PROC_BROWSER_TEST_P(OAuth2Test, PRE_PRE_MergeSession) { +IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_PRE_MergeSession) { SetupGaiaServerForUnexpiredAccount(); SimulateNetworkOnline(); LoginAsExistingUser(); @@ -446,7 +449,7 @@ IN_PROC_BROWSER_TEST_P(OAuth2Test, PRE_PRE_MergeSession) { // MergeSession test is running merge session process for an existing profile // that was generated in PRE_PRE_MergeSession test. -IN_PROC_BROWSER_TEST_P(OAuth2Test, PRE_MergeSession) { +IN_PROC_BROWSER_TEST_F(OAuth2Test, PRE_MergeSession) { SetupGaiaServerForExpiredAccount(); SimulateNetworkOnline(); LoginAsExistingUser(); @@ -462,7 +465,7 @@ IN_PROC_BROWSER_TEST_P(OAuth2Test, PRE_MergeSession) { // MergeSession test is attempting to merge session for an existing profile // that was generated in PRE_PRE_MergeSession test. This attempt should fail // since FakeGaia instance isn't configured to return relevant tokens/cookies. -IN_PROC_BROWSER_TEST_P(OAuth2Test, MergeSession) { +IN_PROC_BROWSER_TEST_F(OAuth2Test, MergeSession) { SimulateNetworkOnline(); content::WindowedNotificationObserver( @@ -532,7 +535,9 @@ class FakeGoogle { } // True if we have already served the test page. - bool IsPageRequested() { return start_event_.IsSignaled(); } + bool IsPageRequested () { + return start_event_.IsSignaled(); + } // Waits until we receive a request to serve the test page. void WaitForPageRequest() { @@ -694,7 +699,7 @@ Browser* FindOrCreateVisibleBrowser(Profile* profile) { return browser; } -IN_PROC_BROWSER_TEST_P(MergeSessionTest, PageThrottle) { +IN_PROC_BROWSER_TEST_F(MergeSessionTest, PageThrottle) { StartNewUserSession(false); // Try to open a page from google.com. @@ -737,7 +742,7 @@ IN_PROC_BROWSER_TEST_P(MergeSessionTest, PageThrottle) { DVLOG(1) << "Loaded page at the end : " << title; } -IN_PROC_BROWSER_TEST_P(MergeSessionTest, XHRThrottle) { +IN_PROC_BROWSER_TEST_F(MergeSessionTest, XHRThrottle) { StartNewUserSession(false); // Wait until we get send merge session request. @@ -791,7 +796,4 @@ IN_PROC_BROWSER_TEST_P(MergeSessionTest, XHRThrottle) { EXPECT_TRUE(fake_google_.IsPageRequested()); } -INSTANTIATE_TEST_CASE_P(OAuth2Suite, OAuth2Test, testing::Bool()); -INSTANTIATE_TEST_CASE_P(MergeSessionSuite, MergeSessionTest, testing::Bool()); - } // namespace chromeos diff --git a/chrome/browser/chromeos/login/startup_utils.cc b/chrome/browser/chromeos/login/startup_utils.cc index c136ea3..1a0d47f 100644 --- a/chrome/browser/chromeos/login/startup_utils.cc +++ b/chrome/browser/chromeos/login/startup_utils.cc @@ -13,7 +13,6 @@ #include "base/sys_info.h" #include "base/threading/thread_restrictions.h" #include "chrome/browser/browser_process.h" -#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/extensions/features/feature_channel.h" #include "chrome/common/pref_names.h" @@ -60,7 +59,7 @@ void StartupUtils::RegisterPrefs(PrefRegistrySimple* registry) { registry->RegisterBooleanPref(prefs::kEnrollmentRecoveryRequired, false); registry->RegisterStringPref(prefs::kInitialLocale, "en-US"); registry->RegisterBooleanPref(prefs::kNewOobe, false); - registry->RegisterBooleanPref(prefs::kWebviewSigninDisabled, false); + registry->RegisterBooleanPref(prefs::kWebviewSigninEnabled, false); } // static @@ -180,41 +179,25 @@ std::string StartupUtils::GetInitialLocale() { // static bool StartupUtils::IsWebviewSigninAllowed() { - return !base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kDisableWebviewSigninFlow); + return extensions::GetCurrentChannel() <= chrome::VersionInfo::CHANNEL_DEV && + !base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kDisableWebviewSigninFlow); } // static bool StartupUtils::IsWebviewSigninEnabled() { - policy::DeviceCloudPolicyManagerChromeOS* policy_manager = - g_browser_process - ? g_browser_process->platform_part() - ->browser_policy_connector_chromeos() - ->GetDeviceCloudPolicyManager() - : nullptr; - - bool is_remora_or_shark_requisition = - policy_manager - ? policy_manager->IsRemoraRequisition() || - policy_manager->IsSharkRequisition() - : false; - - bool is_webview_disabled_pref = g_browser_process->local_state()->GetBoolean( - prefs::kWebviewSigninDisabled); - - // TODO(dzhioev): Re-enable webview signin for remora/shark requisition - // http://crbug.com/464049 - return !is_remora_or_shark_requisition && IsWebviewSigninAllowed() && - !is_webview_disabled_pref; + return IsWebviewSigninAllowed() && + g_browser_process->local_state()->GetBoolean( + prefs::kWebviewSigninEnabled); } // static bool StartupUtils::EnableWebviewSignin(bool is_enabled) { - if (is_enabled && !IsWebviewSigninAllowed()) + if (!IsWebviewSigninAllowed()) return false; - g_browser_process->local_state()->SetBoolean(prefs::kWebviewSigninDisabled, - !is_enabled); + g_browser_process->local_state()->SetBoolean(prefs::kWebviewSigninEnabled, + is_enabled); return true; } diff --git a/chrome/browser/chromeos/login/test/oobe_base_test.cc b/chrome/browser/chromeos/login/test/oobe_base_test.cc index 5aa0eff..5dcc635 100644 --- a/chrome/browser/chromeos/login/test/oobe_base_test.cc +++ b/chrome/browser/chromeos/login/test/oobe_base_test.cc @@ -20,7 +20,6 @@ #include "chrome/common/pref_names.h" #include "chromeos/chromeos_switches.h" #include "chromeos/dbus/fake_shill_manager_client.h" -#include "components/policy/core/common/policy_switches.h" #include "components/user_manager/fake_user_manager.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" @@ -33,19 +32,12 @@ namespace chromeos { -// static -const char OobeBaseTest::kFakeUserEmail[] = "fake-email@gmail.com"; -const char OobeBaseTest::kFakeUserPassword[] = "fake-password"; -const char OobeBaseTest::kFakeSIDCookie[] = "fake-SID-cookie"; -const char OobeBaseTest::kFakeLSIDCookie[] = "fake-LSID-cookie"; - OobeBaseTest::OobeBaseTest() : fake_gaia_(new FakeGaia()), network_portal_detector_(NULL), needs_background_networking_(false), gaia_frame_parent_("signin-frame"), - use_webview_(false), - initialize_fake_merge_session_(true) { + use_webview_(false) { set_exit_when_last_browser_closes(false); set_chromeos_user_ = false; } @@ -58,8 +50,6 @@ void OobeBaseTest::SetUp() { PathService::Get(chrome::DIR_TEST_DATA, &test_data_dir); embedded_test_server()->ServeFilesFromDirectory(test_data_dir); - RegisterAdditionalRequestHandlers(); - embedded_test_server()->RegisterRequestHandler( base::Bind(&FakeGaia::HandleRequest, base::Unretained(fake_gaia_.get()))); @@ -77,19 +67,25 @@ void OobeBaseTest::SetUp() { } bool OobeBaseTest::SetUpUserDataDirectory() { - base::FilePath user_data_dir; - CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)); - base::FilePath local_state_path = - user_data_dir.Append(chrome::kLocalStateFilename); + if (use_webview_) { + // Fake Dev channel to enable webview signin. + scoped_channel_.reset( + new extensions::ScopedCurrentChannel(chrome::VersionInfo::CHANNEL_DEV)); - if (!use_webview()) { - // Set webview disabled flag only when local state file does not exist. + base::FilePath user_data_dir; + CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)); + base::FilePath local_state_path = + user_data_dir.Append(chrome::kLocalStateFilename); + + // Set webview enabled flag only when local state file does not exist. // Otherwise, we break PRE tests that leave state in it. if (!base::PathExists(local_state_path)) { base::DictionaryValue local_state_dict; - - // TODO(nkostylev): Fix tests that fail with webview signin. - local_state_dict.SetBoolean(prefs::kWebviewSigninDisabled, true); + local_state_dict.SetBoolean(prefs::kWebviewSigninEnabled, true); + // OobeCompleted to skip controller-pairing-screen which still uses + // iframe and ends up in a JS error in oobe page init. + // See http://crbug.com/467147 + local_state_dict.SetBoolean(prefs::kOobeComplete, true); CHECK(JSONFileValueSerializer(local_state_path) .Serialize(local_state_dict)); @@ -110,11 +106,6 @@ void OobeBaseTest::SetUpInProcessBrowserTestFixture() { } void OobeBaseTest::SetUpOnMainThread() { - if (initialize_fake_merge_session()) { - fake_gaia_->SetFakeMergeSessionParams(kFakeUserEmail, kFakeSIDCookie, - kFakeLSIDCookie); - } - // Restart the thread as the sandbox host process has already been spawned. embedded_test_server()->RestartThreadAndListen(); @@ -132,7 +123,6 @@ void OobeBaseTest::TearDownOnMainThread() { base::Bind(&chrome::AttemptExit)); content::RunMessageLoop(); } - EXPECT_TRUE(embedded_test_server()->ShutdownAndWaitUntilComplete()); ExtensionApiTest::TearDownOnMainThread(); } @@ -146,14 +136,14 @@ void OobeBaseTest::SetUpCommandLine(base::CommandLine* command_line) { command_line->AppendSwitch(::switches::kDisableBackgroundNetworking); command_line->AppendSwitchASCII(chromeos::switches::kLoginProfile, "user"); - GURL gaia_url = gaia_https_forwarder_->GetURL(std::string()); + GURL gaia_url = gaia_https_forwarder_->GetURL(""); command_line->AppendSwitchASCII(::switches::kGaiaUrl, gaia_url.spec()); command_line->AppendSwitchASCII(::switches::kLsoUrl, gaia_url.spec()); command_line->AppendSwitchASCII(::switches::kGoogleApisUrl, gaia_url.spec()); fake_gaia_->Initialize(); - fake_gaia_->set_issue_oauth_code_cookie(use_webview()); + fake_gaia_->set_issue_oauth_code_cookie(use_webview_); } void OobeBaseTest::InitHttpsForwarders() { @@ -162,9 +152,6 @@ void OobeBaseTest::InitHttpsForwarders() { ASSERT_TRUE(gaia_https_forwarder_->Start()); } -void OobeBaseTest::RegisterAdditionalRequestHandlers() { -} - void OobeBaseTest::SimulateNetworkOffline() { NetworkPortalDetector::CaptivePortalState offline_state; offline_state.status = NetworkPortalDetector::CAPTIVE_PORTAL_STATUS_OFFLINE; @@ -230,32 +217,11 @@ WebUILoginDisplay* OobeBaseTest::GetLoginDisplay() { controller->login_display()); } -void OobeBaseTest::WaitForGaiaPageLoad() { - WaitForSigninScreen(); - - if (!use_webview()) - return; - - ASSERT_TRUE(content::ExecuteScript( - GetLoginUI()->GetWebContents(), - "$('gaia-signin').gaiaAuthHost_.addEventListener('ready'," - "function() {" - "window.domAutomationController.setAutomationId(0);" - "window.domAutomationController.send('GaiaReady');" - "});")); - - content::DOMMessageQueue message_queue; - std::string message; - do { - ASSERT_TRUE(message_queue.WaitForMessage(&message)); - } while (message != "\"GaiaReady\""); -} - void OobeBaseTest::WaitForSigninScreen() { WizardController* wizard_controller = WizardController::default_controller(); - if (wizard_controller) + if (wizard_controller) { wizard_controller->SkipToLoginForTesting(LoginScreenContext()); - + } WizardController::SkipPostLoginScreensForTesting(); login_screen_load_observer_->Wait(); diff --git a/chrome/browser/chromeos/login/test/oobe_base_test.h b/chrome/browser/chromeos/login/test/oobe_base_test.h index 7c2f592..6933a01 100644 --- a/chrome/browser/chromeos/login/test/oobe_base_test.h +++ b/chrome/browser/chromeos/login/test/oobe_base_test.h @@ -34,22 +34,9 @@ class NetworkPortalDetectorTestImpl; // Base class for OOBE, login, SAML and Kiosk tests. class OobeBaseTest : public ExtensionApiTest { public: - // Default fake user email and password, may be used by tests. - - static const char kFakeUserEmail[]; - static const char kFakeUserPassword[]; - - // FakeGaia is configured to return these cookies for kFakeUserEmail. - static const char kFakeSIDCookie[]; - static const char kFakeLSIDCookie[]; - OobeBaseTest(); ~OobeBaseTest() override; - // Subclasses may register their own custom request handlers that will - // process requests prior it gets handled by FakeGaia instance. - virtual void RegisterAdditionalRequestHandlers(); - protected: // InProcessBrowserTest overrides. void SetUp() override; @@ -73,23 +60,12 @@ class OobeBaseTest : public ExtensionApiTest { // Checks JavaScript |expression| in login screen. void JsExpect(const std::string& expression); - bool use_webview() { return use_webview_; } - void set_use_webview(bool use_webview) { use_webview_ = use_webview; } - - bool initialize_fake_merge_session() { - return initialize_fake_merge_session_; - } - void set_initialize_fake_merge_session(bool value) { - initialize_fake_merge_session_ = value; - } - // Returns chrome://oobe WebUI. content::WebUI* GetLoginUI(); // Returns login display. WebUILoginDisplay* GetLoginDisplay(); - void WaitForGaiaPageLoad(); void WaitForSigninScreen(); void ExecuteJsInSigninFrame(const std::string& js); void SetSignFormField(const std::string& field_id, @@ -107,7 +83,6 @@ class OobeBaseTest : public ExtensionApiTest { scoped_ptr gaia_https_forwarder_; std::string gaia_frame_parent_; bool use_webview_; - bool initialize_fake_merge_session_; DISALLOW_COPY_AND_ASSIGN(OobeBaseTest); }; diff --git a/chrome/browser/chromeos/login/webview_login_browsertest.cc b/chrome/browser/chromeos/login/webview_login_browsertest.cc index b7d3caa..c3163b2 100644 --- a/chrome/browser/chromeos/login/webview_login_browsertest.cc +++ b/chrome/browser/chromeos/login/webview_login_browsertest.cc @@ -10,27 +10,57 @@ #include "content/public/test/test_utils.h" namespace chromeos { +namespace { +const char kFakeUserEmail[] = "fake-email@gmail.com"; +const char kFakeUserPassword[] = "fake-password"; +const char kFakeSIDCookie[] = "fake-SID-cookie"; +const char kFakeLSIDCookie[] = "fake-LSID-cookie"; +} class WebviewLoginTest : public OobeBaseTest { public: - WebviewLoginTest() { set_use_webview(true); } + WebviewLoginTest() { use_webview_ = true; } ~WebviewLoginTest() override {} + void SetUpOnMainThread() override { + fake_gaia_->SetFakeMergeSessionParams(kFakeUserEmail, kFakeSIDCookie, + kFakeLSIDCookie); + + OobeBaseTest::SetUpOnMainThread(); + } + void SetUpCommandLine(base::CommandLine* command_line) override { command_line->AppendSwitch(switches::kOobeSkipPostLogin); OobeBaseTest::SetUpCommandLine(command_line); } + void WaitForGaiaPageLoaded() { + WaitForSigninScreen(); + + ASSERT_TRUE(content::ExecuteScript( + GetLoginUI()->GetWebContents(), + "$('gaia-signin').gaiaAuthHost_.addEventListener('ready'," + "function() {" + "window.domAutomationController.setAutomationId(0);" + "window.domAutomationController.send('GaiaReady');" + "});")); + + content::DOMMessageQueue message_queue; + std::string message; + ASSERT_TRUE(message_queue.WaitForMessage(&message)); + EXPECT_EQ("\"GaiaReady\"", message); + } + private: DISALLOW_COPY_AND_ASSIGN(WebviewLoginTest); }; IN_PROC_BROWSER_TEST_F(WebviewLoginTest, Basic) { - WaitForGaiaPageLoad(); + WaitForGaiaPageLoaded(); JsExpect("$('close-button-item').hidden"); - SetSignFormField("identifier", OobeBaseTest::kFakeUserEmail); + SetSignFormField("identifier", kFakeUserEmail); ExecuteJsInSigninFrame("document.getElementById('nextButton').click();"); JsExpect("$('close-button-item').hidden"); @@ -39,21 +69,21 @@ IN_PROC_BROWSER_TEST_F(WebviewLoginTest, Basic) { chrome::NOTIFICATION_SESSION_STARTED, content::NotificationService::AllSources()); - SetSignFormField("password", OobeBaseTest::kFakeUserPassword); + SetSignFormField("password", kFakeUserPassword); ExecuteJsInSigninFrame("document.getElementById('nextButton').click();"); session_start_waiter.Wait(); } IN_PROC_BROWSER_TEST_F(WebviewLoginTest, BackButton) { - WaitForGaiaPageLoad(); + WaitForGaiaPageLoaded(); // Start: no back button, first page. JsExpect("$('back-button-item').hidden"); JsExpect("$('signin-frame').src.indexOf('#identifier') != -1"); // Next step: back button active, second page. - SetSignFormField("identifier", OobeBaseTest::kFakeUserEmail); + SetSignFormField("identifier", kFakeUserEmail); ExecuteJsInSigninFrame("document.getElementById('nextButton').click();"); JsExpect("!$('back-button-item').hidden"); JsExpect("$('signin-frame').src.indexOf('#challengepassword') != -1"); @@ -73,7 +103,7 @@ IN_PROC_BROWSER_TEST_F(WebviewLoginTest, BackButton) { chrome::NOTIFICATION_SESSION_STARTED, content::NotificationService::AllSources()); - SetSignFormField("password", OobeBaseTest::kFakeUserPassword); + SetSignFormField("password", kFakeUserPassword); ExecuteJsInSigninFrame("document.getElementById('nextButton').click();"); session_start_waiter.Wait(); diff --git a/chrome/browser/chromeos/login/wizard_controller_browsertest.cc b/chrome/browser/chromeos/login/wizard_controller_browsertest.cc index 3b71746..658a78a 100644 --- a/chrome/browser/chromeos/login/wizard_controller_browsertest.cc +++ b/chrome/browser/chromeos/login/wizard_controller_browsertest.cc @@ -7,8 +7,6 @@ #include "base/basictypes.h" #include "base/command_line.h" #include "base/compiler_specific.h" -#include "base/json/json_file_value_serializer.h" -#include "base/path_service.h" #include "base/prefs/pref_registry_simple.h" #include "base/prefs/pref_service.h" #include "base/prefs/pref_service_factory.h" @@ -50,7 +48,6 @@ #include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/ui/webui/chromeos/login/oobe_ui.h" #include "chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h" -#include "chrome/common/chrome_constants.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/pref_names.h" @@ -968,11 +965,7 @@ IN_PROC_BROWSER_TEST_F(WizardControllerBrokenLocalStateTest, ASSERT_EQ(1, fake_session_manager_client()->start_device_wipe_call_count()); } -// Boolean parameter is used to run this test for webview (true) and for -// iframe (false) GAIA sign in. -class WizardControllerProxyAuthOnSigninTest - : public WizardControllerTest, - public testing::WithParamInterface { +class WizardControllerProxyAuthOnSigninTest : public WizardControllerTest { protected: WizardControllerProxyAuthOnSigninTest() : proxy_server_(net::SpawnedTestServer::TYPE_BASIC_AUTH_PROXY, @@ -998,32 +991,6 @@ class WizardControllerProxyAuthOnSigninTest proxy_server_.host_port_pair().ToString()); } - bool SetUpUserDataDirectory() override { - base::FilePath user_data_dir; - CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir)); - base::FilePath local_state_path = - user_data_dir.Append(chrome::kLocalStateFilename); - - // Set webview disabled flag only when local state file does not exist. - // Otherwise, we break PRE tests that leave state in it. - if (!base::PathExists(local_state_path)) { - base::DictionaryValue local_state_dict; - - if (!GetParam()) - local_state_dict.SetBoolean(prefs::kWebviewSigninDisabled, true); - - // TODO(paulmeyer): Re-enable webview version of this test - // (drop this condition) once http://crbug.com/452452 is fixed. - if (GetParam()) - local_state_dict.SetBoolean(prefs::kWebviewSigninDisabled, true); - - CHECK(JSONFileValueSerializer(local_state_path) - .Serialize(local_state_dict)); - } - - return WizardControllerTest::SetUpUserDataDirectory(); - } - net::SpawnedTestServer& proxy_server() { return proxy_server_; } private: @@ -1032,7 +999,7 @@ class WizardControllerProxyAuthOnSigninTest DISALLOW_COPY_AND_ASSIGN(WizardControllerProxyAuthOnSigninTest); }; -IN_PROC_BROWSER_TEST_P(WizardControllerProxyAuthOnSigninTest, +IN_PROC_BROWSER_TEST_F(WizardControllerProxyAuthOnSigninTest, ProxyAuthDialogOnSigninScreen) { content::WindowedNotificationObserver auth_needed_waiter( chrome::NOTIFICATION_AUTH_NEEDED, @@ -1044,10 +1011,6 @@ IN_PROC_BROWSER_TEST_P(WizardControllerProxyAuthOnSigninTest, auth_needed_waiter.Wait(); } -INSTANTIATE_TEST_CASE_P(WizardControllerProxyAuthOnSigninSuite, - WizardControllerProxyAuthOnSigninTest, - testing::Bool()); - class WizardControllerKioskFlowTest : public WizardControllerFlowTest { protected: WizardControllerKioskFlowTest() {} diff --git a/chrome/browser/chromeos/policy/blocking_login_browsertest.cc b/chrome/browser/chromeos/policy/blocking_login_browsertest.cc index 03b0504..dabc1f7 100644 --- a/chrome/browser/chromeos/policy/blocking_login_browsertest.cc +++ b/chrome/browser/chromeos/policy/blocking_login_browsertest.cc @@ -12,7 +12,6 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/chromeos/login/existing_user_controller.h" -#include "chrome/browser/chromeos/login/test/oobe_base_test.h" #include "chrome/browser/chromeos/login/ui/webui_login_display.h" #include "chrome/browser/chromeos/login/wizard_controller.h" #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h" @@ -28,6 +27,7 @@ #include "content/public/browser/notification_registrar.h" #include "content/public/browser/notification_service.h" #include "content/public/test/test_utils.h" +#include "google_apis/gaia/fake_gaia.h" #include "google_apis/gaia/gaia_switches.h" #include "google_apis/gaia/gaia_urls.h" #include "net/http/http_status_code.h" @@ -81,35 +81,52 @@ struct BlockingLoginTestParam { }; class BlockingLoginTest - : public OobeBaseTest, + : public InProcessBrowserTest, public content::NotificationObserver, public testing::WithParamInterface { public: BlockingLoginTest() : profile_added_(NULL) {} void SetUpCommandLine(base::CommandLine* command_line) override { - OobeBaseTest::SetUpCommandLine(command_line); - + // Initialize the test server early, so that we can use its base url for + // the command line flags. + ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady()); + + // Use the login manager screens and the gaia auth extension. + command_line->AppendSwitch(switches::kLoginManager); + command_line->AppendSwitch(switches::kForceLoginManagerInTests); + command_line->AppendSwitchASCII(switches::kLoginProfile, "user"); command_line->AppendSwitchASCII(::switches::kAuthExtensionPath, "gaia_auth"); + + // Redirect requests to gaia and the policy server to the test server. + command_line->AppendSwitchASCII(::switches::kGaiaUrl, + embedded_test_server()->base_url().spec()); + command_line->AppendSwitchASCII(::switches::kLsoUrl, + embedded_test_server()->base_url().spec()); command_line->AppendSwitchASCII( policy::switches::kDeviceManagementUrl, embedded_test_server()->GetURL("/device_management").spec()); } void SetUpOnMainThread() override { + fake_gaia_.Initialize(); + + embedded_test_server()->RegisterRequestHandler( + base::Bind(&BlockingLoginTest::HandleRequest, base::Unretained(this))); + embedded_test_server()->RegisterRequestHandler( + base::Bind(&FakeGaia::HandleRequest, base::Unretained(&fake_gaia_))); + registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED, content::NotificationService::AllSources()); - - OobeBaseTest::SetUpOnMainThread(); } void TearDownOnMainThread() override { RunUntilIdle(); EXPECT_TRUE(responses_.empty()); STLDeleteElements(&responses_); - OobeBaseTest::TearDownOnMainThread(); + EXPECT_TRUE(embedded_test_server()->ShutdownAndWaitUntilComplete()); } void Observe(int type, @@ -228,14 +245,10 @@ class BlockingLoginTest } protected: - void RegisterAdditionalRequestHandlers() override { - embedded_test_server()->RegisterRequestHandler( - base::Bind(&BlockingLoginTest::HandleRequest, base::Unretained(this))); - } - Profile* profile_added_; private: + FakeGaia fake_gaia_; std::vector responses_; content::NotificationRegistrar registrar_; diff --git a/chrome/browser/resources/extensions/extension_list.js b/chrome/browser/resources/extensions/extension_list.js index f6967bd..47dd710 100644 --- a/chrome/browser/resources/extensions/extension_list.js +++ b/chrome/browser/resources/extensions/extension_list.js @@ -285,11 +285,32 @@ cr.define('extensions', function() { this.extensions_ = extensions; this.showExtensionNodes_(); resolve(); + + // |onUpdateFinished_| should be called after the list becomes visible + // in extensions.js. |resolve| is async, so |onUpdateFinished_| + // shouldn't be called directly. + this.extensionsUpdated_.then(this.onUpdateFinished_.bind(this)); }.bind(this)); }.bind(this)); return this.extensionsUpdated_; }, + /** Updates elements that need to be visible in order to update properly. */ + onUpdateFinished_: function() { + assert(!this.hidden); + assert(!this.parentElement.hidden); + + this.updateFocusableElements(); + + var idToHighlight = this.getIdQueryParam_(); + if (idToHighlight && $(idToHighlight)) + this.scrollToNode_(idToHighlight); + + var idToOpenOptions = this.getOptionsQueryParam_(); + if (idToOpenOptions && $(idToOpenOptions)) + this.showEmbeddedExtensionOptions_(idToOpenOptions, true); + }, + /** @return {number} The number of extensions being displayed. */ getNumExtensions: function() { return this.extensions_.length; @@ -339,14 +360,6 @@ cr.define('extensions', function() { assertInstanceof(node, ExtensionFocusRow).destroy(); } } - - var idToHighlight = this.getIdQueryParam_(); - if (idToHighlight && $(idToHighlight)) - this.scrollToNode_(idToHighlight); - - var idToOpenOptions = this.getOptionsQueryParam_(); - if (idToOpenOptions && $(idToOpenOptions)) - this.showEmbeddedExtensionOptions_(idToOpenOptions, true); }, /** Updates each row's focusable elements without rebuilding the grid. */ diff --git a/chrome/browser/resources/extensions/extensions.js b/chrome/browser/resources/extensions/extensions.js index 211ecba..3eba3da 100644 --- a/chrome/browser/resources/extensions/extensions.js +++ b/chrome/browser/resources/extensions/extensions.js @@ -263,7 +263,6 @@ cr.define('extensions', function() { var hasExtensions = extensionList.getNumExtensions() != 0; $('no-extensions').hidden = hasExtensions; $('extension-list-wrapper').hidden = !hasExtensions; - $('extension-settings-list').updateFocusableElements(); }.bind(this)); }, diff --git a/chrome/browser/resources/settings/a11y_page/a11y_page.css b/chrome/browser/resources/settings/a11y_page/a11y_page.css new file mode 100644 index 0000000..2277ed3 --- /dev/null +++ b/chrome/browser/resources/settings/a11y_page/a11y_page.css @@ -0,0 +1,23 @@ +/* Copyright 2015 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. */ + +.autoclick-delay-label { + -webkit-margin-end: 0; + -webkit-margin-start: 40px; + margin-bottom: 10px; + margin-top: 0; +} + +.autoclick-dropdown { + -webkit-margin-start: 10px; +} + +.autoclick-dropdown::shadow paper-dropdown-menu { + margin: 0; + padding: 0 0 0.25em; +} + +.more-a11y-link { + margin-bottom: 10px; +} diff --git a/chrome/browser/resources/settings/a11y_page/a11y_page.html b/chrome/browser/resources/settings/a11y_page/a11y_page.html index 8076bbd..cf7a15a 100644 --- a/chrome/browser/resources/settings/a11y_page/a11y_page.html +++ b/chrome/browser/resources/settings/a11y_page/a11y_page.html @@ -5,12 +5,11 @@ -