Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
781163003:
Flip flag to turn CrOS Browser Compositor Single Threaded (Closed)
|
Created:
6 years ago by weiliangc Modified:
5 years, 10 months ago CC:
chromium-reviews, danakj+watch_chromium.org, darin-cc_chromium.org, jam, jbauman+watch_chromium.org, kalyank, piman+watch_chromium.org, sievers+watch_chromium.org, Ian Vollick, enne (OOO), jamesr, jbauman Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFlip flag to turn CrOS Browser Compositor Single Threaded
Simply stop using threaded browser compositor for CrOS. Clean up would
be in follow up patch.
R=danakj@chromium.org, piman@chromium.org
BUG=329552
Committed: https://crrev.com/e111a7d64d9ba12833900ab743beaf4ee6d267ed
Cr-Commit-Position: refs/heads/master@{#313842}
Committed: https://crrev.com/71ed016319424e1e18f7818696320d2894ae724c
Cr-Commit-Position: refs/heads/master@{#314809}
Patch Set 1 #
Total comments: 1
Patch Set 2 : rebase #Patch Set 3 : rm switch #Patch Set 4 : no longer rm switch #
Total comments: 2
Patch Set 5 : leave log fatal as it is #Patch Set 6 : rebase #
Messages
Total messages: 29 (10 generated)
lgtm
Looks like we have some fallout on the windows bots from using scheduler. But LGTM once we sort that out. https://codereview.chromium.org/781163003/diff/1/ui/compositor/test/in_proces... File ui/compositor/test/in_process_context_factory.cc (left): https://codereview.chromium.org/781163003/diff/1/ui/compositor/test/in_proces... ui/compositor/test/in_process_context_factory.cc:76: switches::kUIDisableThreadedCompositing); Not: remove the switch too if these are the only users of it? It'll be easy to forget that later.
weiliangc@chromium.org changed reviewers: + jam@chromium.org
Revive with removing command line switch. jam@chromium.org: Please review changes in chrome/browser/ui/pdf
weiliangc@chromium.org changed reviewers: - jam@chromium.org
weiliangc@chromium.org changed reviewers: + sky@chromium.org
sky@chromium.org: Please review changes in chrome/browser/ui/pdf
weiliangc@chromium.org changed reviewers: - sky@chromium.org
Will delete switch in later CL (probably 779223007). Will check this in first.
The CQ bit was checked by weiliangc@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/781163003/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
https://codereview.chromium.org/781163003/diff/60001/content/browser/composit... File content/browser/compositor/gpu_process_transport_factory.cc (right): https://codereview.chromium.org/781163003/diff/60001/content/browser/composit... content/browser/compositor/gpu_process_transport_factory.cc:239: #if defined(OS_CHROMEOS) Actually, it looks like browser_tests on linux_chromeos run without the thread and in software. Leave this part as is then? (or remove LOG?)
Updated. https://codereview.chromium.org/781163003/diff/60001/content/browser/composit... File content/browser/compositor/gpu_process_transport_factory.cc (right): https://codereview.chromium.org/781163003/diff/60001/content/browser/composit... content/browser/compositor/gpu_process_transport_factory.cc:239: #if defined(OS_CHROMEOS) On 2015/01/29 23:56:25, piman (Very slow to review) wrote: > Actually, it looks like browser_tests on linux_chromeos run without the thread > and in software. > Leave this part as is then? (or remove LOG?) Leaving it as it is now.
The CQ bit was checked by weiliangc@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/781163003/80001
The CQ bit was unchecked by weiliangc@chromium.org
The CQ bit was checked by weiliangc@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/781163003/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/e111a7d64d9ba12833900ab743beaf4ee6d267ed Cr-Commit-Position: refs/heads/master@{#313842}
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/893623002/ by weiliangc@chromium.org. The reason for reverting is: Possible cause for veyron: Chrome crash in SW Mirror mode (at login screen) https://code.google.com/p/chromium/issues/detail?id=453732 .
Message was sent while issue was closed.
Reason for revert (crbug.com/453732) is fixed by crrev.com/899073003, so will re-land this.
The CQ bit was checked by weiliangc@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/781163003/100001
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/71ed016319424e1e18f7818696320d2894ae724c Cr-Commit-Position: refs/heads/master@{#314809} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
