-
Notifications
You must be signed in to change notification settings - Fork 211
[Android] Fix a set of rendering issues introduced by single process mod... #49
[Android] Fix a set of rendering issues introduced by single process mod... #49
Conversation
Performance compare with ContentShell: |
The pull request fixes the following P1 high bugs |
Trybot DashboardBuilding status:Finished the build of the PATCH in this pull requestFri Aug 30 2013 20:59:12 GMT+0800 (CST) [content_linux] Failed LOGS: failed update Rebuild the failed bots you want!Select the bots:
Start the building:
|
lgtm, This is very critical to android port because we use in process rendering mode. This patch can fix all black screen issues. @darktears please help review. |
@kenchris please have a check. we need to merge it before this friday because it'll block all testing next week. |
@sqliu , I don't quite understand why duplicating gl context class implementation could resolve the race condition. In your description, what's the resource be raced? Could you please explain more about the root cause? Thanks. |
@huningxin in single process mode the "g_all_shared_contexts" is shared by both browser thread and renderer thread, and thus the command buffer is shared, this leads to race conditions and black screen. The reason I duplicate the implementation is that this fix is ralatively simple and can minimize the effort of code change, otherwise, changes lies in almost everywhere and makes rebase a big effort. |
@sqliu , thanks for the explanation. I am just concerning that by duplicating the two classes, we also need to track and sync with the upstream change (we may not even know upstream changed). If your solution is to use different |
@huningxin Yes, that also a solution, but the problem is |
After talked with @sqliu , I got to understand the tradeoff we made. I am OK with this workaround for critical issue for upcoming code opening. We can investigate how to make it more maintainable and maybe work with upstream after that. BTW, you need to rebase the patch. Thanks. |
We recognize the signin page by: - in the browser on the UI thread, checking the renderer's process_id against SigninManager::signin_process_id. - in the browser on the IO thread, checking the renderer's process_id against ExtensionInfoMap::signin_process_id (a cache of SigninManager's value). - in the renderer, checking for a --signin_process command line flag. This means we can still allow extensions on non-chrome-signin pages hosted on accounts.google.com. BUG=220039 TEST=In a fresh Chrome profile, install the extension listed in bug comment crosswalk-project#49. Click Hotdog menu -> Sign in to Chrome. The signin page should not be redirected away from the accounts.google.com page, and it should not have a red background. But if you navigate directly to accounts.google.com, it *should* either be redirected or have a red background. Review URL: https://chromiumcodereview.appspot.com/15030008 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@201444 0039d316-1c4b-4281-b951-d872f2087c98
…mode. * There is browser side compositor in Android port, in single process mode. the gl contexts in renderer will have race conditions with gl contexts in browser side. * This issue leads to many rendering failes: (1) stop activity and then restart will cause black screen; (2) Any page with compositing layers will leads to the compositing layer black (WebGL. Canvas, Video). * Fix this issue by duplicating the gl contexts implementation in browser side. * This is a workaround fix for the critical blocking issue. BUG=crosswalk-project/crosswalk#449, 430, 447, 455
@huningxin Patch rebased. This is a workaround fix, and we need to find a better fix to this issue. But for now we need to merge it ASAP for 9/10 release. @kenchris @darktears @yongsheng @gramakri-intel PTAL, thanks! |
@huningxin thanks a lot for understanding the current dilemma. We need a formal solution later. |
@sqliu wow, huge implementation. I appreciate for your effort and knowledge. |
This is gigantic, and going to be a pain to maintain because :
Again If we land that today for having the QA to run (and potentially the next milestone), I want somebody assigned today with an issue number to solve that correctly either upstream, either with a less intrusive patch. And that work should start right now, the more we wait the more the code will diverge there, the more pain it's going to be. It's dead important to take care of this over some other feature of Crosswalk : the maintenance, the possibility to move to newer Chrome exceeds by far many of the features we working on. |
crosswalk-project/crosswalk#516 Please assign someone to it even before the first milestone. |
Clicking the button makes me sad :( |
[Android] Fix a set of rendering issues introduced by single process mod...
…hset #5 id:100001 of https://codereview.chromium.org/516293007/) Reason for revert: Chrome OS build with DCHECKs enabled fails on start. This CL breaks assumption, that GAIA extension can be loaded without performing IO-operations. Log and stack trace: [16275:16275:0903/161435:WARNING:renderer_freezer.cc(55)] Cgroup freezer does not exist or is not writable. Processes will not be frozen during suspend. [16275:16275:0903/161435:WARNING:configuration_policy_pref_store.cc(30)] Policy RemoteAccessClientFirewallTraversal: This policy has been deprecated. [16275:16275:0903/161437:FATAL:thread_restrictions.cc(38)] Function marked as IO-only was called from a thread that disallows IO! If this thread really should be allowed to make IO calls, adjust the call to base::ThreadRestrictions::SetIOAllowed() in this thread's startup. #0 0x7fe3122e555e base::debug::StackTrace::StackTrace() #1 0x7fe31237c462 logging::LogMessage::~LogMessage() #2 0x7fe3124824af base::ThreadRestrictions::AssertIOAllowed() #3 0x7fe3123630a1 base::PathExists() #4 0x7fe321cfd5f3 extensions::(anonymous namespace)::CollectPlatformSpecificResourceArchs() #5 0x7fe321cfad22 extensions::Extension::InitFromValue() #6 0x7fe321cfa490 extensions::Extension::Create() #7 0x7fe321cfa114 extensions::Extension::Create() #8 0x7fe3233ebe1b extensions::ComponentLoader::Load() #9 0x7fe3233ec3cf extensions::ComponentLoader::Add() #10 0x7fe3233ec32a extensions::ComponentLoader::Add() #11 0x7fe3233ec2b0 extensions::ComponentLoader::Add() #12 0x7fe32437505f (anonymous namespace)::LoadGaiaAuthExtension() #13 0x7fe324374d5e extensions::GaiaAuthExtensionLoader::LoadIfNeeded() #14 0x7fe324237f7e ScopedGaiaAuthExtension::ScopedGaiaAuthExtension() #15 0x7fe322f68bb2 chromeos::WebUILoginView::Init() #16 0x7fe322f556ef chromeos::LoginDisplayHostImpl::InitLoginWindowAndView() #17 0x7fe322f52640 chromeos::LoginDisplayHostImpl::LoadURL() #18 0x7fe322f5226a chromeos::LoginDisplayHostImpl::StartWizard() #19 0x7fe322f54acd chromeos::LoginDisplayHostImpl::StartPostponedWebUI() #20 0x7fe322f541f8 chromeos::LoginDisplayHostImpl::Observe() #21 0x7fe322f54bfd chromeos::LoginDisplayHostImpl::Observe() #22 0x7fe31b222377 content::NotificationServiceImpl::Notify() #23 0x7fe322d51a24 chromeos::(anonymous namespace)::UserWallpaperDelegate::OnWallpaperAnimationFinished() #24 0x7fe31188692f ash::RootWindowController::OnWallpaperAnimationFinished() #25 0x7fe3117c633a ash::(anonymous namespace)::ShowWallpaperAnimationObserver::OnImplicitAnimationsCompleted() #26 0x7fe311f72d30 ui::ImplicitAnimationObserver::CheckCompleted() #27 0x7fe311f72cd5 ui::ImplicitAnimationObserver::SetActive() #28 0x7fe311f965d5 ui::ScopedLayerAnimationSettings::~ScopedLayerAnimationSettings() #29 0x7fe3117c5ec7 ash::DesktopBackgroundWidgetController::StartAnimating() #30 0x7fe3117bbb0c ash::DesktopBackgroundController::InstallDesktopController() #31 0x7fe3117bbc03 ash::DesktopBackgroundController::InstallDesktopControllerForAllWindows() #32 0x7fe3117bb3cc ash::DesktopBackgroundController::SetDesktopBackgroundImageMode() #33 0x7fe3117bb096 ash::DesktopBackgroundController::SetWallpaperImage() #34 0x7fe322f9a6b9 chromeos::WallpaperManager::DoSetDefaultWallpaper() #35 0x7fe322f99b73 chromeos::WallpaperManager::PendingWallpaper::ProcessRequest() #36 0x7fe322fbac72 base::internal::RunnableAdapter<>::Run() #37 0x7fe322fbabe9 base::internal::InvokeHelper<>::MakeItSo() #38 0x7fe322fbaba5 base::internal::Invoker<>::Run() #39 0x7fe3122cc9ce base::Callback<>::Run() #40 0x7fe312488f86 base::Timer::RunScheduledTask() #41 0x7fe3124890bc base::BaseTimerTaskInternal::Run() #42 0x7fe312489382 base::internal::RunnableAdapter<>::Run() #43 0x7fe3124892ec base::internal::InvokeHelper<>::MakeItSo() #44 0x7fe312489295 base::internal::Invoker<>::Run() #45 0x7fe3122cc9ce base::Callback<>::Run() #46 0x7fe3122eb9b3 base::debug::TaskAnnotator::RunTask() #47 0x7fe3123a1c57 base::MessageLoop::RunTask() #48 0x7fe3123a1d9b base::MessageLoop::DeferOrRunPendingTask() #49 0x7fe3123a228d base::MessageLoop::DoDelayedWork() #50 0x7fe3122a1f25 base::MessagePumpGlib::Run() #51 0x7fe3123a17f0 base::MessageLoop::RunHandler() #52 0x7fe3124093b2 base::RunLoop::Run() #53 0x7fe324217e4d ChromeBrowserMainParts::MainMessageLoopRun() #54 0x7fe31ad2a8bf content::BrowserMainLoop::RunMainMessageLoopParts() #55 0x7fe31ad344c7 content::BrowserMainRunnerImpl::Run() #56 0x7fe31ad251b1 content::BrowserMain() #57 0x7fe31abb997f content::RunNamedProcessTypeMain() #58 0x7fe31abbcce8 content::ContentMainRunnerImpl::Run() #59 0x7fe31abb8ee5 content::ContentMain() #60 0x7fe3206d3505 ChromeMain #61 0x7fe3206d34b2 main Cannot upload crash dump: cannot exec /sbin/crash_reporter Crash_reporter failed to process crash report Original issue's description: > Enable forced extension updates on NaCl arch mismatch > > This makes extensions aware of the platforms for which > they have platform-specific resources installed, if any. > > This also hooks up the extension update code with some > additional logic to place an extension in forced-update > mode if it has platform-specific resources which don't > match the current NaCl architecture. > > BUG=409948 > TEST=install an extension which uses NaCl (QuickOffice for example). Rename the _platform-specific/<your-nacl-arch> directory some something else and force an update (e.g. via chrome://extensions button). Observe that a new CRX is downloaded and installed. > > Committed: https://chromium.googlesource.com/chromium/src/+/4a92281fa5d331860d65a59ba45dc882a5c71df4 [email protected],[email protected],[email protected],[email protected] NOTREECHECKS=true NOTRY=true BUG=409948 Review URL: https://codereview.chromium.org/532183003 Cr-Commit-Position: refs/heads/master@{#293128}
…::ExecuteCommands. (patchset #1 id:1 of https://codereview.chromium.org/547783002/) Reason for revert: Broke Linux ChromiumOS Tests (dbg)(3) http://build.chromium.org/p/chromium.webkit/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%283%29/builds/1206 BrowserTestBase signal handler received SIGTERM. Backtrace: #0 0x7f3bcf3aa67e base::debug::StackTrace::StackTrace() #1 0x0000041c884a content::(anonymous namespace)::DumpStackTraceSignalHandler() #2 0x7f3bc709f4a0 <unknown> #3 0x7f3bc7151a43 __poll #4 0x7f3bc7b8dff6 <unknown> #5 0x7f3bc7b8e124 g_main_context_iteration #6 0x7f3bcf366f35 base::MessagePumpGlib::Run() #7 0x7f3bcf468180 base::MessageLoop::RunHandler() #8 0x7f3bcf4cfcc2 base::RunLoop::Run() #9 0x0000041e0729 content::RunThisRunLoop() #10 0x0000041e06ba content::RunMessageLoop() #11 0x000000668feb ExtensionApiTest::ResultCatcher::GetNextResult() #12 0x0000006a30dd extensions::CommandsApiTest_ContinuePropagation_Test::RunTestOnMainThread() #13 0x0000006a3712 extensions::CommandsApiTest_ContinuePropagation_Test::RunTestOnMainThread() #14 0x00000176be8b InProcessBrowserTest::RunTestOnMainThreadLoop() #15 0x0000041c85c8 content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() #16 0x0000041c9cf2 base::internal::RunnableAdapter<>::Run() #17 0x0000041c9c69 base::internal::InvokeHelper<>::MakeItSo() #18 0x0000041c9c25 base::internal::Invoker<>::Run() #19 0x00000063f0de base::Callback<>::Run() #20 0x0000042dddd2 ChromeBrowserMainParts::PreMainMessageLoopRunImpl() #21 0x0000042dca92 ChromeBrowserMainParts::PreMainMessageLoopRun() #22 0x000002e77daf chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun() #23 0x7f3bd7bd2e96 content::BrowserMainLoop::PreMainMessageLoopRun() #24 0x7f3bd7bd96f2 base::internal::RunnableAdapter<>::Run() #25 0x7f3bd7bd965c base::internal::InvokeHelper<>::MakeItSo() #26 0x7f3bd7bd960a base::internal::Invoker<>::Run() #27 0x7f3bd80a852e base::Callback<>::Run() #28 0x7f3bd841bd2b content::StartupTaskRunner::RunAllTasksNow() #29 0x7f3bd7bd12f0 content::BrowserMainLoop::CreateStartupTasks() #30 0x7f3bd7bdcc72 content::BrowserMainRunnerImpl::Initialize() #31 0x7f3bd7bcda55 content::BrowserMain() #32 0x7f3bd7a6224f content::RunNamedProcessTypeMain() #33 0x7f3bd7a655b8 content::ContentMainRunnerImpl::Run() #34 0x7f3bd7a617b5 content::ContentMain() #35 0x0000041c82cb content::BrowserTestBase::SetUp() #36 0x00000176afc3 InProcessBrowserTest::SetUp() #37 0x000000674823 ExtensionBrowserTest::SetUp() #38 0x000000674852 ExtensionBrowserTest::SetUp() #39 0x0000018298a3 testing::internal::HandleSehExceptionsInMethodIfSupported<>() #40 0x00000181686e testing::internal::HandleExceptionsInMethodIfSupported<>() #41 0x00000180ad73 testing::Test::Run() #42 0x00000180b4cb testing::TestInfo::Run() #43 0x00000180baca testing::TestCase::Run() #44 0x000001811009 testing::internal::UnitTestImpl::RunAllTests() #45 0x0000018225b3 testing::internal::HandleSehExceptionsInMethodIfSupported<>() #46 0x0000018185fe testing::internal::HandleExceptionsInMethodIfSupported<>() #47 0x000001810c97 testing::UnitTest::Run() #48 0x0000041a5531 RUN_ALL_TESTS() #49 0x0000041a4547 base::TestSuite::Run() #50 0x0000008026d2 InteractiveUITestSuiteRunner::RunTestSuite() #51 0x000001769618 (anonymous namespace)::ChromeTestLauncherDelegate::RunTestSuite() #52 0x0000041d9efb content::LaunchTests() #53 0x00000176954b LaunchChromeTests() #54 0x00000080262f main #55 0x7f3bc708a76d __libc_start_main #56 0x0000005ffda9 <unknown> [123/334] CommandsApiTest.ContinuePropagation (TIMED OUT) Original issue's description: > Fixes HasEventListener check in ExtensionKeybindingRegistry::ExecuteCommands. > > ExtensionKeybindingRegistry::ExecuteCommands functions in two distinct ways -- execute all commands based on an accelerator, or, execute an accelerator for a given extension. The former behavior is implied by passing an empty string. > > Previously, only the latter case was handled when trying to continue propagating keys. > > BUG=407163 > TEST=try bots (interactive_ui_tests --gtest_filter=CommandsApiTest.*). > > Committed: https://chromium.googlesource.com/chromium/src/+/189a2ed4d209231d517b0e64a722722dead3f17a [email protected],[email protected] NOTREECHECKS=true NOTRY=true BUG=407163 Review URL: https://codereview.chromium.org/552533003 Cr-Commit-Position: refs/heads/master@{#293780}
- Do not hide the 'scan new card' popup on every character typed. - Show 'scan new card' only when user is typing digits or separators. - Hide 'scan new card' suggestion after user typed in 6 digits. BUG=449646, 441536 Review URL: https://codereview.chromium.org/857633003 Cr-Commit-Position: refs/heads/master@{#312009} (cherry picked from commit 25f305d) Review URL: https://codereview.chromium.org/856993002 Cr-Commit-Position: refs/branch-heads/2272@{#49} Cr-Branched-From: 827a380-refs/heads/master@{#310958}
…hrottler Merge to M42. This fixes a use-after-destroy bug leading to crashes. Object Lifetimes: blink::WebPlugin >= PluginInstanceThrottler >= PluginPreroller. The PluginPreroller is supposed to observe the Throttler and destroy itself when the Throttler gets destroyed. Previously, the WebPlugin would sometimes destroy the Throttler before the PluginPreroller was ever created. The PluginPreroller would then try to observe a non-existent throttler, and crash. This patch now attaches the PluginPreroller to the Throttler before the blink::WebPlugin has a chance to destroy the Throttler. BUG=459920, 458326, 403800 Review URL: https://codereview.chromium.org/941803003 Cr-Commit-Position: refs/heads/master@{#317837} (cherry picked from commit 260ce72) Review URL: https://codereview.chromium.org/962313002 Cr-Commit-Position: refs/branch-heads/2311@{#49} Cr-Branched-From: 09b7de5-refs/heads/master@{#317474}
…custom cursor. TODO(oshima): support custom cursor. BUG=476078 TEST=verify that file manager shows the default large pointer cursor when cropping images. [email protected] [email protected] NOTRY=true NOPRESUBMIT=true Review URL: https://codereview.chromium.org/1081513003 Cr-Commit-Position: refs/heads/master@{#324701} (cherry picked from commit c86ec52) Review URL: https://codereview.chromium.org/1073413002 Cr-Commit-Position: refs/branch-heads/2357@{#49} Cr-Branched-From: 59d4494-refs/heads/master@{#323860}
This fixes two places where we directly access property tree indices rather than using their accessors. [email protected] BUG=489725 Review URL: https://codereview.chromium.org/1144343002 Cr-Commit-Position: refs/heads/master@{#330818} (cherry picked from commit dc1cefd) Review URL: https://codereview.chromium.org/1148293003 Cr-Commit-Position: refs/branch-heads/2403@{#49} Cr-Branched-From: f54b809-refs/heads/master@{#330231}
When the user clicks on the lock icon in a Custom Tab, the click listener creates a new piece of UI. This inflation fails if the context passed to WebSiteSettingsPopup#show() is not an Activity. BUG=524005 Review URL: https://codereview.chromium.org/1318483002 Cr-Commit-Position: refs/heads/master@{#345858} (cherry picked from commit 63c9943) Review URL: https://codereview.chromium.org/1315713004 . Cr-Commit-Position: refs/branch-heads/2490@{#49} Cr-Branched-From: 7790a35-refs/heads/master@{#344925}
…id:1 of https://codereview.chromium.org/1590863004/ ) Reason for revert: Reverting since it's causing a MediaPlayer to be created along side the WebMediaPlayerImpl instance... which causes playback to start twice :-/ Original issue's description: > Enable MediaSession with the WMPI Cast player. > > Looks like the cast impl didn't enable MediaSession when cast is > not present. Tweaked the code a bit so that it's available. > > BUG=575276,529887 > TEST=MediaSession play/pause work. > > Committed: https://crrev.com/0ae23b0efd1eb826fe3df2af5baa9415c828b593 > Cr-Commit-Position: refs/heads/master@{#369895} [email protected] BUG=575276,529887 Review URL: https://codereview.chromium.org/1615513003 Cr-Commit-Position: refs/heads/master@{#370624} (cherry picked from commit 6e2716d) Review URL: https://codereview.chromium.org/1617953002 . Cr-Commit-Position: refs/branch-heads/2623@{#49} Cr-Branched-From: 92d7753-refs/heads/master@{#369907}
This isn't a real fix for Dill, but it stops the crash. BUG=591213 TBR=ianwen Review URL: https://codereview.chromium.org/1763463002 (cherry picked from commit 579efe8) Cr-Original-Commit-Position: refs/heads/master@{#378875} Cr-Commit-Position: refs/branch-heads/2661@{crosswalk-project#49} Cr-Branched-From: ef6f6ae-refs/heads/master@{#378081}
MediaCodecPlayer expects the demuxer response for each demuxer data request. Currently it might happen that no response would come after a data request, which might result in the player lockup. This situation might happen if the data request callback arrives late at the Media thread, e.g. after the player started the Seek sequence. In this case the data request could come after the demuxer seek request which violates the demuxer contract. This CL checks the decoder state before calling the data request from demuxer. BUG=557334 [email protected] > Review URL: https://codereview.chromium.org/1455853002 > Cr-Commit-Position: refs/heads/master@{#360392} Review URL: https://codereview.chromium.org/1457713003 . Cr-Commit-Position: refs/branch-heads/2564@{crosswalk-project#49} Cr-Branched-From: 1283eca-refs/heads/master@{#359700}
…nts. BUG=612501 Review-Url: https://codereview.chromium.org/2002643002 Cr-Commit-Position: refs/heads/master@{#395556} (cherry picked from commit d7185e0) Review URL: https://codereview.chromium.org/2012723002 . Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#49} Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
JellyBean crashes if a header is added to a ListView after the adapter has been set. Switch the order of adding the header and setting the adapter. BUG=761726 [email protected] Change-Id: I1cfafe3b437667469d28fc9f3cc79ed0137a1faf Reviewed-on: https://chromium-review.googlesource.com/650626 Reviewed-by: Matthew Jones <[email protected]> Commit-Queue: Theresa <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#499674} Reviewed-on: https://chromium-review.googlesource.com/653419 Reviewed-by: Theresa <[email protected]> Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#49} Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
...e.
gl contexts in renderer will have race conditions with gl contexts in browser side.
will cause black screen; (2) Any page with compositing layers will leads to the
compositing layer black(WebGL, Canvas, Video).
BUG=crosswalk-project/crosswalk#449, 430, 447, 455