Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

‘usenewprocess‘ attribute for webview allowing to create new process for every webview. #2

Closed
wants to merge 31 commits into from

Conversation

janRucka
Copy link
Contributor

Reasons for pull request:

In our project we need to use multiple webviews and share cookies. Also we need new process for every webview. If webviews share process then if one webview is slow-down (rendering new page or even frozen for whatever reason) the other is slow-down as well. Only way (or best way as far as I know) to avoid this is to use new process.

In nw12 we used different partition ID for every webview. Then every webview is run in new process. But this is not a possibility for us anymore since cookies are not shared (if different partition ID is used). I think the behavior in nw12 was more or less buggy because is different from what is described here: https://developer.chrome.com/apps/tags/webview#partition

Here is bachel complain about running in one process as well:
nwjs/nw.js#3622 (comment)

Solution:

We added ‘usenewprocess’ attribute to webview. If this attribute is used then new process for every webview should be started. If attribute is not used behavior should remain the same as is now (new process only if different partition ID is used).

Hence change will only affect users which for whatever reason need to use new process for every webview (and will use ‘usenewprocess’ attribute). Others will not be affected at all.

Examples:

Working only with our chanches in code.
twoWebviewsOneProcess.zip
twoWebviewsTwoProcesses.zip

@rogerwang rogerwang force-pushed the nw13 branch 3 times, most recently from 9285daa to f255843 Compare March 3, 2016 00:17
rogerwang pushed a commit that referenced this pull request Mar 3, 2016
… (patchset #2 id:20001 of https://codereview.chromium.org/1730153005/ )

Reason for revert:
The reason for reverting is: Speculative revert for Compile failure on Win, Mac
& Precise 64 Beta Bots (crbug.com/590286)

Original issue's description:
> [cherrypick] Fix computation of runtime for throttled tasks
>
> The computation of the delayed task runtime was computing the (virtual)
> TimeDomain's now + delay.  That doesn't take into account the skew
> between the real time and the virtual time, which meant timers could
> run sooner than expected for background tabs.  Timers associated with
> foreground tabs are unaffected.
>
> This patch makes the computation of delayed runtime  the responsibility
> of the TimeDomain and introduces a ThrottledTimeDomain which computes
> the delayed runtime as realtime + delay.
>
> This means timers will no longer run sooner than expected.  Under normal
> circumstancs this only timers whose delay modulo 1000ms is less than 100
> ms are less likely to be affected.  Timers whose whose delay modulo
> 1000ms is greater than 900 are highly likely to be affected (there's a
> good chance their delay was too small before).
>
> BUG=587074
>
> Review URL: https://codereview.chromium.org/1718233002
>
> Cr-Commit-Position: refs/heads/master@{#376993}
> (cherry picked from commit b56bbe4)
>
> R=skyostil@chromium.org
>
> Committed: https://chromium.googlesource.com/chromium/src/+/fd563254490ae93382cb88a1e9a1c1e54815c97b

TBR=skyostil@chromium.org,alexclarke@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=587074

Review URL: https://codereview.chromium.org/1743813002

Cr-Commit-Position: refs/branch-heads/2623@{#526}
Cr-Branched-From: 92d7753-refs/heads/master@{#369907}
rogerwang pushed a commit that referenced this pull request Mar 3, 2016
The computation of the delayed task runtime was computing the (virtual)
TimeDomain's now + delay.  That doesn't take into account the skew
between the real time and the virtual time, which meant timers could
run sooner than expected for background tabs.  Timers associated with
foreground tabs are unaffected.

This patch makes the computation of delayed runtime  the responsibility
of the TimeDomain and introduces a ThrottledTimeDomain which computes
the delayed runtime as realtime + delay.

This means timers will no longer run sooner than expected.  Under normal
circumstancs this only timers whose delay modulo 1000ms is less than 100
ms are less likely to be affected.  Timers whose whose delay modulo
1000ms is greater than 900 are highly likely to be affected (there's a
good chance their delay was too small before).

BUG=587074

Review URL: https://codereview.chromium.org/1718233002

Cr-Commit-Position: refs/heads/master@{#376993}
(cherry picked from commit b56bbe4)

R=skyostil@chromium.org

Review URL: https://codereview.chromium.org/1751953002 .

Cr-Commit-Position: refs/branch-heads/2623@{#550}
Cr-Branched-From: 92d7753-refs/heads/master@{#369907}
-Attribute 'usenewprocess' added on webview. If used it ensure that webview is started in new process. If not used chrome will decide whether or not new process will be used.
@janRucka
Copy link
Contributor Author

janRucka commented Mar 4, 2016

Did you have time to review this Pull request? It is there already for 10 days without any reaction. What is the most annoying for me is to do constant merging with every new commit in nw13 branch. Your workflow (to create patch with changes on actual chromium version) make merge even more time consuming.

@rogerwang
Copy link
Member

We was in the middle of rebasing to Chromium 49. Will review soon.

@janRucka
Copy link
Contributor Author

janRucka commented Mar 4, 2016

Thanks

jtg-gg referenced this pull request in jtg-gg/chromium.src Mar 7, 2016
Cr-Commit-Position: refs/branch-heads/2564@{#2}
Cr-Branched-From: 1283eca-refs/heads/master@{#359700}
rogerwang pushed a commit that referenced this pull request Feb 27, 2017
…rd navigation.'

Cherry-picked patchset #1 and #2 from cl/2691693002

BUG=688047, 677356
Review-Url: https://codereview.chromium.org/2685803002
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2711123004
Cr-Commit-Position: refs/branch-heads/2987@{#666}
Cr-Branched-From: ad51088-refs/heads/master@{#444943}
rogerwang pushed a commit that referenced this pull request Apr 7, 2017
…chset #2 id:20001 of https://codereview.chromium.org/2666893002/ )

Reason for revert:
First, this has a race: CONTEXT_KEY_HAS_GAIA_ACCOUNT can (and actually does) come after CONTEXT_KEY_PROFILE_PICTURE_DATA_URL thus triggering exception:

login.js:1208 Uncaught TypeError: Cannot read property 'title' of undefined
    at HTMLUnknownElement.updateItem (login.js:1208)
    at Array.<anonymous> (login.js:9504)
    at ScreenContext.applyChanges (login.js:538)
    at HTMLDivElement.contextChanged_ (login.js:329)
    at Object.api.contextChanged (login.js:424)
    at <anonymous>:1:23

Second, it breaks supervised user creation screen (and probably other screens that share user-image-grid object).

BUG=697842,682236

Original issue's description:
> Delete "Google Profile photo" for the Active Directory
>
> Delete "Google Profile photo" choice from the image choice screen for
> the Active Directory users.
>
> BUG=682236
> TEST=manual
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
>
> Review-Url: https://codereview.chromium.org/2666893002
> Cr-Commit-Position: refs/heads/master@{#447513}
> Committed: https://chromium.googlesource.com/chromium/src/+/33f901b2ff76e1d08d80e3ba5d1d079178cb4978

TBR=achuith@chromium.org,rsorokin@chromium.org
BUG=682236
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2727953004
Cr-Commit-Position: refs/heads/master@{#454525}
(cherry picked from commit ed71ca7)

Review-Url: https://codereview.chromium.org/2785593002 .
Cr-Commit-Position: refs/branch-heads/3029@{#466}
Cr-Branched-From: 939b32e-refs/heads/master@{#454471}
GnorTech pushed a commit that referenced this pull request Apr 26, 2017
Cr-Commit-Position: refs/branch-heads/3071@{#2}
Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
GnorTech pushed a commit that referenced this pull request Apr 26, 2017
… (patchset #4 id:170001 of https://codereview.chromium.org/2809043004/ )

Reason for revert:
Causes http://crbug.com/714386

Original issue's description:
> Reland of Initialize default audio device ID with explicit device ID. (patchset #1 id:1 of https://codereview.chromium.org/2813543005/ )
>
> Reason for revert:
> Will attempt to reland by reverting changes in MSM and updating extensions test.
>
> Original issue's description:
> > Revert of Initialize default audio device ID with explicit device ID. (patchset #2 id:20001 of https://codereview.chromium.org/2812903002/ )
> >
> > Reason for revert:
> > Patchset 2 restores the behavior I wanted to eliminate.
> >
> > Original issue's description:
> > > Initialize default audio device ID with explicit device ID.
> > >
> > > This is more consistent with how video device IDs are specified and is
> > > also a small first step towards implementing the standard constraints
> > > algorithm for audio.
> > > This CL disables a misleading selector that allows switching the
> > > user-preferred device in the middle of a getUserMedia call.
> > > Since that selector does not affect the current getUserMedia call, it
> > > is better to have it disabled, which is already the case with video
> > > devices.
> > >
> > > BUG=708081
> > >
> > > Review-Url: https://codereview.chromium.org/2812903002
> > > Cr-Commit-Position: refs/heads/master@{#463624}
> > > Committed: https://chromium.googlesource.com/chromium/src/+/82382cca8c9aa804acb7fd9cfaa6e4478f92cd7d
> >
> > TBR=hbos@chromium.org
> > # Skipping CQ checks because original CL landed less than 1 days ago.
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG=708081
> >
> > Review-Url: https://codereview.chromium.org/2813543005
> > Cr-Commit-Position: refs/heads/master@{#463634}
> > Committed: https://chromium.googlesource.com/chromium/src/+/cf8ea8d590f1c89c2633c793a41bd31a85b3fe72
>
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=708081
>
> Review-Url: https://codereview.chromium.org/2809043004
> Cr-Commit-Position: refs/heads/master@{#463710}
> Committed: https://chromium.googlesource.com/chromium/src/+/be26518ed4e8a2966c919a93b488689374b17d42

TBR=mek@chromium.org
BUG=708081,714386

Review-Url: https://codereview.chromium.org/2829403002
Cr-Commit-Position: refs/heads/master@{#466550}
(cherry picked from commit 18633fc)

Review-Url: https://codereview.chromium.org/2837873004 .
Cr-Commit-Position: refs/branch-heads/3071@{#194}
Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
rogerwang pushed a commit that referenced this pull request May 9, 2017
…set #2 id:20001 of https://codereview.chromium.org/2671433002/ )

Reason for revert:
This breaks Chrome iOS dashboards because cpu_architecture is the only field where we have the device model info (e.g. iPhone7,2).

Original issue's description:
> Don't set cpu architecture field on iOS in UMA logs.
>
> The value we were setting it to before was not correct, so this
> change instead just makes the value not get set on iOS.
>
> BUG=370104
>
> Review-Url: https://codereview.chromium.org/2671433002
> Cr-Commit-Position: refs/heads/master@{#447578}
> Committed: https://chromium.googlesource.com/chromium/src/+/a86e8e115054ad167458d0379980db5473cb20de

TBR=rkaplow@chromium.org,asvitkine@chromium.org
BUG=370104, 718080

Review-Url: https://codereview.chromium.org/2855943005
Cr-Commit-Position: refs/heads/master@{#469062}
(cherry picked from commit 9f89092)

Review-Url: https://codereview.chromium.org/2863143002 .
Cr-Commit-Position: refs/branch-heads/3029@{#803}
Cr-Branched-From: 939b32e-refs/heads/master@{#454471}
rogerwang pushed a commit that referenced this pull request May 17, 2017
…set #2 id:20001 of https://codereview.chromium.org/2671433002/ )

Reason for revert:
This breaks Chrome iOS dashboards because cpu_architecture is the only field where we have the device model info (e.g. iPhone7,2).

Original issue's description:
> Don't set cpu architecture field on iOS in UMA logs.
>
> The value we were setting it to before was not correct, so this
> change instead just makes the value not get set on iOS.
>
> BUG=370104
>
> Review-Url: https://codereview.chromium.org/2671433002
> Cr-Commit-Position: refs/heads/master@{#447578}
> Committed: https://chromium.googlesource.com/chromium/src/+/a86e8e115054ad167458d0379980db5473cb20de

TBR=rkaplow@chromium.org,asvitkine@chromium.org
BUG=370104, 718080

Review-Url: https://codereview.chromium.org/2855943005
Cr-Commit-Position: refs/heads/master@{#469062}
(cherry picked from commit 9f89092)

Review-Url: https://codereview.chromium.org/2860093004 .
Cr-Commit-Position: refs/branch-heads/3071@{#416}
Cr-Branched-From: a106f0a-refs/heads/master@{#464641}
rogerwang pushed a commit that referenced this pull request Jul 17, 2017
…irst use. (patchset #2 id:40001 of https://codereview.chromium.org/2871843003/ )

Reason for revert:
The perf regression was not caused by this piece of code. Reverting to the original behavior.

See https://docs.google.com/document/d/1eeTSJg18rJ4v_oxxSJU8MK20UywnEhocWk_WrWv8kQE/edit for details of the regression investigation.

Original issue's description:
> [translate] Defer start of translate ranker model load to first use.
>
> This CL is in response to perf regression (see referenced bugs).
>
> BUG=697947, 697665
>
> Review-Url: https://codereview.chromium.org/2871843003
> Cr-Commit-Position: refs/heads/master@{#471086}
> Committed: https://chromium.googlesource.com/chromium/src/+/d2876f4ee1c1a01ee262ca55752503ba3a30a6d5

TBR=benhenry@chromium.org,vmiura@chromium.org,sullivan@chromium.org,tdresser@chromium.org,groby@chromium.org,rogerm@chromium.org
BUG=697947, 697665

Review-Url: https://codereview.chromium.org/2918173002
Cr-Original-Commit-Position: refs/heads/master@{#478379}
Review-Url: https://codereview.chromium.org/2956053003 .
Cr-Commit-Position: refs/branch-heads/3112@{#481}
Cr-Branched-From: b6460e2-refs/heads/master@{#474897}
rogerwang pushed a commit that referenced this pull request Jul 20, 2017
This reverts commit 1b60006.

Reason for revert:
Causes a startup crash with chrome --mash, possibly because mash does not have a DeviceDataManager. You might want to check with sadrul@ or rjkroege@ about whether there's a good way to do this under mash.

Failing build (chromeos-side waterfall):
https://luci-milo.appspot.com/buildbot/chromeos.chrome/tricky-tot-chrome-pfq-informational/5205

Stack (from running manually with chrome --mash on Linux):

[60089:60089:0717/114019.863351:FATAL:device_data_manager.cc(84)] Check failed: instance_. DeviceDataManager was not created.
#0 0x7f06a03468bc base::debug::StackTrace::StackTrace()
#1 0x7f06a036a121 logging::LogMessage::~LogMessage()
#2 0x7f069aeb08ff ui::DeviceDataManager::GetInstance()
#3 0x55f183402a41 chromeos::LoginDisplayHostImpl::LoginDisplayHostImpl()
#4 0x55f1834066ad chromeos::ShowLoginWizard()
#5 0x55f1833e4bf7 chromeos::ChromeSessionManager::Initialize()
#6 0x55f1832f9cb4 chromeos::ChromeBrowserMainPartsChromeos::PostProfileInit()
#7 0x55f1837e7aa2 ChromeBrowserMainParts::PreMainMessageLoopRunImpl()
#8 0x55f1837e709d ChromeBrowserMainParts::PreMainMessageLoopRun()
#9 0x55f1832f8f4a chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun()
#10 0x7f069d9e5081 content::BrowserMainLoop::PreMainMessageLoopRun()
#11 0x7f069dde6407 content::StartupTaskRunner::RunAllTasksNow()
#12 0x7f069d9e353b content::BrowserMainLoop::CreateStartupTasks()
#13 0x7f069d9e7a72 content::BrowserMainRunnerImpl::Initialize()
#14 0x7f069d9e0b57 content::BrowserMain()
#15 0x7f069e1e8462 content::ContentMainRunnerImpl::Run()
#16 0x7f06a0890d39 service_manager::Main()
#17 0x7f069e1e7384 content::ContentMain()
#18 0x55f182e8132f ChromeMain
#19 0x7f069454af45 __libc_start_main
#20 0x55f182e81194 <unknown>

Original change's description:
> Listen to changes to touch input devices
>
> In https://codereview.chromium.org/2964823002 the OobeDisplayChooser
> started using the DeviceDataManager to look for touchscreen devices when
> searching for a good primary display to use during OOBE.
>
> On device cold boot the DeviceDataManager has not yet found any
> touchscreen devices at the time OobeUi::ShowOobeUI() is called (likely
> due to lower level systems not being fully initialized).
>
> This CL make LoginDisplayHostImpl an observer of changes to connected
> touchscreen devices, re-triggering the OobeDisplayChooser when the
> DeviceDataManager is notified of the connected touchscreens. This
> overcomes the timing issues on cold boot.
>
> Bug: 738885
> Change-Id: Iae488ddc9428b7c5e74d36cf18e35ba3d1235bbd
> Reviewed-on: https://chromium-review.googlesource.com/569958
> Reviewed-by: Jacob Dufault <jdufault@chromium.org>
> Commit-Queue: Felix Ekblom <felixe@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#487007}

TBR=alemate@chromium.org,jdufault@chromium.org,felixe@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

(cherry picked from commit 0cd134a)

Bug: 738885
Change-Id: If31322734e679bbb1f4eef0a9aa802d34263cba4
Reviewed-on: https://chromium-review.googlesource.com/574731
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#487191}
Signed-off-by: Bernie Thompson <bhthompson@google.com>
Reviewed-on: https://chromium-review.googlesource.com/575149
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Cr-Commit-Position: refs/branch-heads/3112@{#623}
Cr-Branched-From: b6460e2-refs/heads/master@{#474897}
GnorTech pushed a commit that referenced this pull request Aug 5, 2017
TBR=dimu@chromium.org

Change-Id: Ifbffc39df9024c37350df83abc7b715e52d277f9
Reviewed-on: https://chromium-review.googlesource.com/580748
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#2}
Cr-Branched-From: ff259ba-refs/heads/master@{#488528}
rogerwang pushed a commit that referenced this pull request Aug 17, 2017
changes:
new specs require removing suggested apps indicator, specs:
peeking: https://screenshot.googleplex.com/QNw4VjLHD5e
fullscreen page #1: https://screenshot.googleplex.com/OT0NmfnByWg
fullscreen page #2: https://screenshot.googleplex.com/7PC0iytVXHq

fullscreen page#01: https://screenshot.googleplex.com/NJiyTFFzqKW
fullscreen page#02: https://screenshot.googleplex.com/8Dh1Z1YC3aA

TBR=warx@chromium.org

(cherry picked from commit dce53c5)

screenshot: 
peeking: https://screenshot.googleplex.com/tMxgChqhrSm
Test: test with fullscreen app list flag enabled
Bug: 750292
Change-Id: I841342e95101e54ae7e022383ecbfbe56c503e2d
Reviewed-on: https://chromium-review.googlesource.com/606997
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Qiang(Joe) Xu <warx@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#492775}
Reviewed-on: https://chromium-review.googlesource.com/611300
Reviewed-by: Vadim Tryshev <vadimt@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#468}
Cr-Branched-From: ff259ba-refs/heads/master@{#488528}
rogerwang pushed a commit that referenced this pull request Aug 27, 2017
It's possible for BrowserAssociatedInterface's InternalState to have
Initialize invoked after ClearBindings, which can in turn lead to a
violation of the assumption that ClearBindings ensures no future message
dispatches to the bound implementation. This can occur in the following
rare but plausible scenario for a type X which inherits
BrowserAssociatedInterface<T>:

  1. Post task to IO thread which may destroy some yet-uncreated
     instance x of type X (e.g. maybe it posts a render process ID
     and we'll later have an X associated with that RPH)
  2. Create x as an instance of type X (posts an IO thread task to
     Initialize x's Internal State)
  3. Interface request is received, posting an IO thread task to bind a
     handle to x.
  3. Task from #1 executes, locating and deleting x; resetting
     InternalState bindings (not yet initialized anyway).
  4. Task from #2 executes, initializing the InternalState bindings.
  5. Task from #3 executes, bindings a handle to the InternalState
  6. Message is received on the bound handle, dispatched to deleted x
  7. UAF!

As it turns out, AssociatedBindingSet does not own any thread-affine
state upon default construction; therefore it can be created on any
thread as long as it's subsequently used and destroyed from a single
thread. Since InternalState already guarantees those conditions, this
CL simply removes the async Initialize step.

BUG=753672
R=jam@chromium.org
TBR=rockot@chromium.org

(cherry picked from commit 201a233)

Change-Id: Id603fb57d412daf3741b61f7857a29edeaac5443
Reviewed-on: https://chromium-review.googlesource.com/610924
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#493797}
Reviewed-on: https://chromium-review.googlesource.com/617516
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#606}
Cr-Branched-From: ff259ba-refs/heads/master@{#488528}
GnorTech pushed a commit that referenced this pull request Sep 14, 2017
TBR=dimu@chromium.org

Change-Id: Ia12160b836ddc9bfbcf7cfe2ccc709ef0002779a
Reviewed-on: https://chromium-review.googlesource.com/647208
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3202@{#2}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
GnorTech pushed a commit that referenced this pull request Oct 27, 2017
TBR=dimu@chromium.org

Change-Id: Ibf4495b3dddcfadd1e7a79ecd906d9ccaad72759
Reviewed-on: https://chromium-review.googlesource.com/718136
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#2}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}
GnorTech pushed a commit that referenced this pull request Dec 14, 2017
When using --isolate-origins=... or  --site-per-process, some or all sites will
be given dedicated processes.  Documents (HTML, XML, and some text
files) and other opaque formats (JSON) from such sites should not be
delivered to cross-site pages in the renderer process unless they are made
available via CORS or another exception.

This CL adds a ResourceHandler to enforce this restriction in the browser
process.  The handler inspects the response headers and decides whether
to block it from reaching the renderer process.  It also sniffs the content to
confirm the response is labeled correctly, to avoid blocking things like
JavaScript mislabeled as HTML or JSON.

Blocked responses are sent back as empty response bodies rather than as
a network error, to ensure existing behavior does not change.  Other
cross-site responses (e.g., images, scripts, etc) are still allowed.

Note that the current approach assumes the renderer process is not
compromised and will not lie about the initiating origin, etc.  The need for
assumption will be removed in future work, for bug 268640.
For more details, see:
http://www.chromium.org/developers/design-documents/blocking-cross-site-documents

BUG=786505
TBR=creis@chromium.org

(cherry picked from commit 358baf4)

(cherry picked from commit eff7652786019f8a24edd269867223ee12df7389)

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Reviewed-on: https://chromium-review.googlesource.com/783826
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#522016}
Reviewed-on: https://chromium-review.googlesource.com/818491
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Original-Commit-Position: refs/branch-heads/3239_84@{#2}
Cr-Original-Branched-From: 8f51ed0-refs/branch-heads/3239@{#643}
Cr-Original-Branched-From: adb61db-refs/heads/master@{#508578}
Change-Id: I7f7b78714dd36727ccf41be79f666a61f174995c
Reviewed-on: https://chromium-review.googlesource.com/823283
Cr-Commit-Position: refs/branch-heads/3239@{#668}
Cr-Branched-From: adb61db-refs/heads/master@{#508578}
rogerwang pushed a commit that referenced this pull request Jan 29, 2018
TBR=dimu@chromium.org

Change-Id: I2b2fcc1d7b8f6dd22fdc7d807428f109d30adbf1
Reviewed-on: https://chromium-review.googlesource.com/874902
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#2}
Cr-Branched-From: bc084a8-refs/heads/master@{#530369}
GnorTech pushed a commit that referenced this pull request Mar 20, 2018
TBR=dimu@chromium.org

Change-Id: Ib15c0a5aeb49f1134f0984dbecbaf4f9185e04ac
Reviewed-on: https://chromium-review.googlesource.com/945449
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3359@{#2}
Cr-Branched-From: 66afc5e-refs/heads/master@{#540276}
rogerwang pushed a commit that referenced this pull request Apr 25, 2018
Bug:831762,783383

TBR=chrishtr@chromium.org

(cherry picked from commit 787bde4)

Change-Id: I3aea3afb2669f83f0d40674218c82a4f399b4e6f
Reviewed-on: https://chromium-review.googlesource.com/1008270
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550518}
Reviewed-on: https://chromium-review.googlesource.com/1012570
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#2}
Cr-Branched-From: 9ef2aa8-refs/heads/master@{#550428}
rogerwang pushed a commit that referenced this pull request Jun 19, 2018
TBR=govind@chromium.org

Change-Id: Ic0dfa4e19ec9328a162a535ad1990cab3ce69996
Reviewed-on: https://chromium-review.googlesource.com/1072325
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#2}
Cr-Branched-From: 010ddcf-refs/heads/master@{#561733}
GnorTech pushed a commit that referenced this pull request Aug 4, 2018
TBR=mmoss@chromium.org

Change-Id: I9321fc52b8b4731488b4a3ce2dc7214e6448f784
Reviewed-on: https://chromium-review.googlesource.com/1144494
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#2}
Cr-Branched-From: 271eaf5-refs/heads/master@{#576753}
rogerwang pushed a commit that referenced this pull request Sep 15, 2018
TBR=mmoss@chromium.org

Change-Id: I5488772ea9b1e107a39dd776258413833ad56ac8
Reviewed-on: https://chromium-review.googlesource.com/1198315
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#2}
Cr-Branched-From: 79f7c91-refs/heads/master@{#587811}
rogerwang pushed a commit that referenced this pull request Sep 17, 2018
This CL is a collection of cherry-picks related to crbug.com/868592 fix.
Specifically, this is:
- The original mega-patch crrev.com/222c9ba7c6
- creis@ follow-up fix crrev.com/27986c7c955
- kouhei@ follow-up fix crrev.com/6be8b5a07bdf

The original change descriptions are captured below % Change-Id lines

---

Speculative crash fix for navigator.serviceworker access during unload

This should fix crash/caab6eb137e58385

This CL addresses the unhandled case in crrev.com/582126

TBR=falken@chromium.org

Bug: 881126, 868592
Reviewed-on: https://chromium-review.googlesource.com/1207781
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#589419}(cherry picked from commit 6be8b5a)
Reviewed-on: https://chromium-review.googlesource.com/1212368
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/branch-heads/3545@{#2}
Cr-Branched-From: a2bbe9d-refs/heads/master@{#589377}

Speculative fix for additional History DocumentLoader crashes.

There is no guarantee that the DocumentLoader is always attached [1],
so let's introduce null checks in StateInternal and setScrollRestoration.

[1] The DocumentLoader may be detached while FrameLoader::PrepareForCommit.

BUG=879477, 872672

Reviewed-on: https://chromium-review.googlesource.com/1200075
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588227}
Speculative fix for History::ScrollRestorationInternal null deref

This is a speculative fix for crash reported on crbug.com/872672 .

There is no guarantee that the DocumentLoader is always attached [1],
so let's introduce a null check.

[1] The DocumentLoader may be detached while FrameLoader::PrepareForCommit.

Bug: 872672
Reviewed-on: https://chromium-review.googlesource.com/1171972
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582509}
(cherry picked from commit a2a107d)

NavigatorServiceWorker: Avoid instantiating if being navigated away.

This CL fixes a clusterfuzz crash which fails to minimize.

Bug: 872320
Reviewed-on: https://chromium-review.googlesource.com/1170160
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582126}
(cherry picked from commit d995adf)

Flush microtask queue before commit

Bug: 868592
Reviewed-on: https://chromium-review.googlesource.com/1164148
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581124}
(cherry picked from commit 38894f6)

Prevent promise reject to be sync scheduled during DocumentLoader detach
(% mod: revert fetch_manager.cc change)

(cherry picked from commit c415acc)

Bug: 868592
Change-Id: I50029416f0441a9f09c538716684a01cb8af93e1
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1163235
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/master@{#580814}
Reviewed-on: https://chromium-review.googlesource.com/1184122
Cr-Original-Commit-Position: refs/branch-heads/3497@{#760}
Cr-Original-Branched-From: 271eaf5-refs/heads/master@{#576753}
Reviewed-on: https://chromium-review.googlesource.com/1218183
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#938}
Cr-Branched-From: 271eaf5-refs/heads/master@{#576753}
@rogerwang
Copy link
Member

In the new site isolation architecture in upstream, webview now runs in separate process. So I'm closing this.

@rogerwang rogerwang closed this Sep 26, 2018
rogerwang pushed a commit that referenced this pull request Oct 31, 2018
TBR=benmason@chromium.org

Change-Id: Ic56dc53e7abf1fdb6a44a10e31ebfc53aecd86e6
Reviewed-on: https://chromium-review.googlesource.com/c/1278330
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#2}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
rogerwang pushed a commit that referenced this pull request Nov 8, 2018
Pressing the skip button leads to two LoginDisplayHost::Finalize() calls, which
causes a double deletion eventually leading to a crash.

The PIN setup skip button triggers two 'discover-done' events (fired in
discover_ui.js::end_), and the observer in screen_discover.js sends a finished
user action whenver it receives a discover-done event.

Discover should not be sending discover-done twice. This CL just prevents the crash.

Finalize #1
    at HTMLElement.end_
    at HTMLElement.fire
    at HTMLElement.onSkipButton_
    at HTMLElement.onAuthTokenChanged_
    at HTMLElement._observerEffect
    at HTMLElement._effectEffects
    at HTMLElement._propertySetter
    at HTMLElement.setter
    at HTMLElement.onSkipButton_
    at HTMLElement.handler

Finalize #2
    at HTMLElement.end_
    at HTMLElement.fire
    at HTMLElement.onSkipButton_
    at HTMLElement.handler
    at HTMLElement.fire
    at Object.fire
    at Object.forward
    at Object.click
    at HTMLElement.handleNative

TBR=jdufault@google.com

(cherry picked from commit 41254eb)

Bug: 895250
Change-Id: I784ee5ef0ecfc5bffad515973451331bfc0aee3f
Reviewed-on: https://chromium-review.googlesource.com/c/1299145
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602880}
Reviewed-on: https://chromium-review.googlesource.com/c/1308414
Reviewed-by: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#409}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
rogerwang pushed a commit that referenced this pull request Nov 17, 2018
…Mojave

This change is required due to new security restrictions in Mojave.  We can no longer
inject input w/o being added as an accessibility app in the security applet.

While this sounds like a usefulk speedbump, it causes remote access applications quite
a bit of trouble:
1.) We don't use the restricted API until a user connects so they cannot approve remotely
2.) The dialog appears to only show up once (regardless of approve/deny status)
3.) Users connecting to a locked machine will never see the dialog

This is affecting quite a few CRD users, basically everyone who upgrades to Mojave
will experience this one way or another.  This is the simplest fix (and easiest to merge)
that I could think of to unblock users.  The prompt will only be shown on 10.14+ platforms
and the request is only shown if the app has not been approved.  I'd like to look at the
user feedback after releasing this change to see if more work is needed.

One problem I anticipate is that the dialog shown doesn't have a lot of context and it
refers to the wrapper script (org.chromium.chromoting.me2me.sh) instead of Chrome Remote
Desktop.  If this is confusing, we can wrap the prompt request in a dialog where we control
the text.  My concern with checking in the feature first is that the new strings won't be
available for merging.

Another behavior to call about this impl is that the prompt will be displayed in two instances:
1.) When the host is first started (choosing enable via app/website)
2.) When the user signs in and the host service is started

Scenario #2 will have less context but that is the only way to ask for permission for
users who upgraded and had CRD installed previously.  The dialog is not displayed at the login
screen (i.e. when no one is signed in).

One last note, there is no way that I can see to specify this permission in the manifest or
set up via a script / at install time.  It requires a user action to complete.

Bug: 901021

Change-Id: I54247dd56111feee9608e1b50387cb3501a659a4
Reviewed-on: https://chromium-review.googlesource.com/c/1315468
Reviewed-by: Gary Kacmarcik <garykac@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#604947}(cherry picked from commit 19e52db)
Reviewed-on: https://chromium-review.googlesource.com/c/1315706
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#478}
Cr-Branched-From: 4226ddf-refs/heads/master@{#599034}
rogerwang pushed a commit that referenced this pull request Dec 18, 2018
TBR=benmason@chromium.org

Change-Id: Ic479ff470024e8f324896b71e9a89153da3a2350
Reviewed-on: https://chromium-review.googlesource.com/c/1356267
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#2}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
rogerwang pushed a commit that referenced this pull request Dec 18, 2018
Background:
My first attempt [1] to implement KeyboardFocusableScrollers
made scrollers click-focusable. This got reported as a
regression in Issue 908809.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1258331.

This happens when HandleMouseFocus() uses SupportsFocus():

Element::SupportsFocus() <---
Element::IsMouseFocusable()
Element::IsFocusable()
MouseEventManager::HandleMouseFocus()
EventHandler::HandleMousePressEvent()

Problem:
After [1], SupportsFocus() and therefore also IsMouseFocusable()
returns true for all scrollable scrollers. If a clicked element
is "mouse focusable", HandleMouseFocus() calls FocusController::
SetFocusedElement(element) which makes |element| the activeElement.
Changing activeElement on clicks is reported as a regression.

Solution:
This CL reverts [1]'s change in SupportFocus() and instead
decouples IsMouseFocusable() and IsKeyboardFocusable().

Now we're back to the old mouse focus semantics: clicking a
scroller does *not* make it the activeElement, does not trigger
:focus CSS and does not target the scroller with a focus-event.
However, tabbing to a scroller *does* all that. That's the
feature of KeyboardFocusableScrollers.

This CL does not add any new tests, instead, it reverts some
of [1]'s test changes so tests again expect that clicking or
tapping a scroller doesn't move focus.

Bug: 585413, 908809
Change-Id: Ia0f41cb2e0bf2c093eb2c6d6fa15af5d00aa69c9
Reviewed-on: https://chromium-review.googlesource.com/c/1354450
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Hugo Holgersson <hugoh@vewd.com>
Cr-Original-Commit-Position: refs/heads/master@{#613920}(cherry picked from commit d01359a)
Reviewed-on: https://chromium-review.googlesource.com/c/1370171
Cr-Commit-Position: refs/branch-heads/3626@{#223}
Cr-Branched-From: d897fb1-refs/heads/master@{#612437}
rogerwang pushed a commit that referenced this pull request Feb 10, 2019
TBR=mmoss@chromium.org

Change-Id: Ib6b8d1056f9600b2a553341d5a95cafa8a4f2f01
Reviewed-on: https://chromium-review.googlesource.com/c/1436410
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#2}
Cr-Branched-From: e510299-refs/heads/master@{#625896}
rogerwang pushed a commit that referenced this pull request Mar 24, 2019
TBR=mmoss@chromium.org

Change-Id: I5645af3484987dd8bf93bd2efab40f9854fdc98a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1511013
Reviewed-by: chrome-release-bot@chromium.org <chrome-release-bot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3729@{#2}
Cr-Branched-From: d4a8972-refs/heads/master@{#638880}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants