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.

Sign up for GitHub

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

Jump to bottom

App exits when accessing 'send to devices' in share menu #34636

Closed
Uni-verse opened this issue Nov 30, 2023 · 4 comments · Fixed by brave/brave-core#21168
Closed

App exits when accessing 'send to devices' in share menu #34636

Uni-verse opened this issue Nov 30, 2023 · 4 comments · Fixed by brave/brave-core#21168
Assignees
@AlexeyBarabash
Labels
crash feature/sync OS/Android Fixes related to Android browser functionality QA Pass - Android ARM QA Pass - Android Tab QA Pass - Android 8+ QA/Test-All-Platforms QA/Yes release-notes/exclude
Milestone
1.61.x - Release

Comments

@Uni-verse
Copy link
Contributor

Description

Crash when accessing 'send to your devices' in share menu

Reproducible on existing Profile & fresh Profile (with sync enabled)

Steps to reproduce

  1. Join sync chain with some devices
  2. Open any url
  3. Open hamburger menu -> share -> send to devices

Actual result

Crash / App exist

send_to_device_crash_01.mp4

Expected result

App should not quit

Issue reproduces how often

Easily

Version/Channel Information:

  • Can you reproduce this issue with the current Play Store version? No
  • Can you reproduce this issue with the current Play Store Beta version? Yes
  • Can you reproduce this issue with the current Play Store Nightly version? Yes

Device details

  • Install type (ARM, x86): ARM
  • Device type (Phone, Tablet, Phablet): ARM + ARM Tablet. Samsung GS 21
  • Android version: 13, 14

Brave version

Brave 1.62.92 Chromium: 120.0.6099.35 (Official Build) beta (64-bit)

@Uni-verse Uni-verse added crash feature/sync QA/Yes release-notes/exclude OS/Android Fixes related to Android browser functionality labels Nov 30, 2023
@hffvld
Copy link
Contributor

hffvld commented Nov 30, 2023

I checked with the latest C119 build 1.62.85 and the issue is not reproducible, then I checked with the first C120 build 1.62.86 and observed the issue.

@AlexeyBarabash AlexeyBarabash mentioned this issue Nov 30, 2023
Merged
25 tasks
@Uni-verse Uni-verse mentioned this issue Nov 30, 2023
Closed
AlexeyBarabash added a commit to brave/brave-core that referenced this issue Dec 1, 2023
@AlexeyBarabash
Fixed crash accessing 'send to devices' in share menu via Sync
20ebc61 
Fixes brave/brave-browser#34636

Related Chromium commit:
https://source.chromium.org/chromium/chromium/src/+/84ed1f62ecd64bbfc3d2c4775a6e80a6b0e9defb

Clean up SendTabToSelfSigninPromo flag

Enabled by default in M119 (https://crrev.com/c/4905024), can be
cleaned up in M120.

Fixed: 1479693, 1295204
Change-Id: Ic778e9c886a5d9391fb5da7510d166952481fe21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4905343
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/main@{#1204655}
AlexeyBarabash added a commit to brave/brave-core that referenced this issue Dec 1, 2023
@AlexeyBarabash
Fixed crash accessing 'send to devices' in share menu via Sync (#21168)
693e8ae 
Fixes brave/brave-browser#34636

Related Chromium commit:
https://source.chromium.org/chromium/chromium/src/+/84ed1f62ecd64bbfc3d2c4775a6e80a6b0e9defb

Clean up SendTabToSelfSigninPromo flag

Enabled by default in M119 (https://crrev.com/c/4905024), can be
cleaned up in M120.

Fixed: 1479693, 1295204
Change-Id: Ic778e9c886a5d9391fb5da7510d166952481fe21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4905343
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/main@{#1204655}
@brave-builds brave-builds added this to the 1.63.x - Nightly milestone Dec 1, 2023
@AlexeyBarabash AlexeyBarabash mentioned this issue Dec 1, 2023
Merged
7 tasks
@AlexeyBarabash AlexeyBarabash self-assigned this Dec 1, 2023
mkarolin pushed a commit to brave/brave-core that referenced this issue Dec 1, 2023
@AlexeyBarabash @mkarolin
Fixed crash accessing 'send to devices' in share menu via Sync (#21168)
9ed657c 
Fixes brave/brave-browser#34636

Related Chromium commit:
https://source.chromium.org/chromium/chromium/src/+/84ed1f62ecd64bbfc3d2c4775a6e80a6b0e9defb

Clean up SendTabToSelfSigninPromo flag

Enabled by default in M119 (https://crrev.com/c/4905024), can be
cleaned up in M120.

Fixed: 1479693, 1295204
Change-Id: Ic778e9c886a5d9391fb5da7510d166952481fe21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4905343
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/main@{#1204655}
@mkarolin mkarolin mentioned this issue Dec 1, 2023
Merged
25 tasks
@kjozwiak kjozwiak mentioned this issue Dec 1, 2023
Open
@kjozwiak
Copy link
Member

kjozwiak commented Dec 1, 2023

The above will be uplifted into 1.61.x via brave/brave-core#21135 hence not adding the required version for verification on 1.62.x. However, the above was verified on Nightly via brave/brave-core#21168 (comment) before uplifting.

kjozwiak pushed a commit to brave/brave-core that referenced this issue Dec 1, 2023
@mkarolin @cdesouza-chromium
@emerick @samartnik @AlexeyBarabash
Upgrade from Chromium 119 to Chromium 120 (uplift to 1.61.x). (#21135)
587ce3e 
* Merge pull request #20434 from brave/cr120

Upgrade from Chromium 119 to Chromium 120.

* Updated chrome-VERSION.patch from Chromium 119.0.6045.163 to Chromium 120.0.6099.35.

* Additional assert_ts.js -> assert.js for 1.61.x.

Chromium change:

https://chromium.googlesource.com/chromium/src/+/f7a2ea671005530e7a891bba411a34bfd1b3b4cd

commit f7a2ea671005530e7a891bba411a34bfd1b3b4cd
Author: dpapad <dpapad@chromium.org>
Date:   Mon Oct 16 21:13:04 2023 +0000

    WebUI: Rename assert_ts.ts to assert.ts, part 7 (last)

    In this part updating one remaining reference in ui/file_manager/ and
    removing assert_ts.ts.

    assert_ts.ts used the "_ts" suffix to differentiate from the previously
    existing assert.js file. The latter no longer exists and the "_ts"
    suffix can be dropped.

    Fixed: 1291526

* Remove `SidePanelCoordinator` `final` keyword substitution (#20765)

`SidePanelCoordinator` is tagged upstream as `final`, and as a
workaround to be able to derive from it, an override had been introduced
at some point to replace the keyword `final` with naught. This type of
redefinition is unreliable, as it is UB, and it also becomes highly
viral.

This change replaces the substitution with a patch. The substitution was
removing the `final` keyword from the signature of other functions in
different headers, and leaving it with no token to indicate the function
is an override, causing a build failure.

* `Label::CalculatePreferredSize()` marked as `final` (#20767)

This function is now marked as `final`, however there is another entry
in the overload set that can be used in its place for the override in
`BraveVPNStatusLabel`.

Chromium change:
https://chromium.googlesource.com/chromium/src/+/bf2f84a8c4f27e399e05bf4730a29152f6a8b0b6

commit bf2f84a8c4f27e399e05bf4730a29152f6a8b0b6
Author: weidongliu <liuwd8@gmail.com>
Date:   Mon Oct 30 15:59:03 2023 +0000

    views: Mark CalculatePreferredSize() of views::Label as final

    Views::Label now has two functions to calculate the preferred size. This
    can cause some unexpected errors. CalculatePreferredSize() is now
    expected to be deprecated. Therefore, mark it as final to prevent
    overrided by subclasses.

    Bug: 1346889

* Revert "Disable kPowerBookmarksSidePanel feature flag"

This reverts commit 237341e.

* Java files formatting.

* Fix `BraveBrowsingDataRemoverDelegate` substitution (#21034)

This override has been missing a matching header since it was
introduced, and this has been causing linking to fail in every upstream
occurrence of `ChromeBrowsingDataRemoverDelegateFactory::GetForProfile`
as the return type in the header was not being replaced accordingly.

This changes adds the changes to the header as well, and provides an
inclusion `BraveBrowsingDataRemoverDelegate` in the shadow file, to
allow the compiler to understand that
`BraveBrowsingDataRemoverDelegate*` is pefectly convertible to
`ChromeBrowsingDataRemoverDelegate`.

* [Android] Move JNI classes to jni_zero namespace

* Disabled failing upstream browser tests.

* Merge pull request #21152 from brave/maxk-fix-page-info-cookies-separator

[cr120 follow-up] Removes an extra separator in cookies page info view.

* Merge pull request #21153 from brave/maxk-redirect-on-device-data-to-cookies

[cr120 follow-up] Redirect On-device data link to cookies page.

* Merge pull request #21166 from brave/maxk-hide-hover-card-image-toggle

[cr120 follow-up] Hide Settings->Appearance hover card toggle.

* Fixed crash accessing 'send to devices' in share menu via Sync (#21168)

Fixes brave/brave-browser#34636

Related Chromium commit:
https://source.chromium.org/chromium/chromium/src/+/84ed1f62ecd64bbfc3d2c4775a6e80a6b0e9defb

Clean up SendTabToSelfSigninPromo flag

Enabled by default in M119 (https://crrev.com/c/4905024), can be
cleaned up in M120.

Fixed: 1479693, 1295204
Change-Id: Ic778e9c886a5d9391fb5da7510d166952481fe21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4905343
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Victor Vianna <victorvianna@google.com>
Cr-Commit-Position: refs/heads/main@{#1204655}

* Merge pull request #21169 from brave/120.0.6099.56_master

Upgrade from Chromium 120.0.6099.35 to Chromium 120.0.6099.56.

* Updated chrome-VERSION.patch from Chromium 120.0.6099.35 to Chromium 120.0.6099.56.

---------

Co-authored-by: cdesouza-chromium <cdesouza@brave.com>
Co-authored-by: Emerick Rogul <erogul@brave.com>
Co-authored-by: Artem Samoilenko <artem@brave.com>
Co-authored-by: AlexeyBarabash <AlexeyBarabash@users.noreply.github.com>
@kjozwiak
Copy link
Member

kjozwiak commented Dec 1, 2023

The above requires 1.61.94 or higher for 1.61.x verification 👍

@hffvld hffvld added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Dec 4, 2023
@hffvld
Copy link
Contributor

hffvld commented Dec 4, 2023

Verified on Pixel 2 XL, Galaxy Tab S8 and Pixel 7 using version(s):

Device/OS: 
- Pixel 2 XL / taimen-user  8.1.0 OPM2.171026.006.H1 release-keys
- Galaxy Tab S8 / gts8wifixx-user 13 TP1A.220624.014 release-keys
- Pixel 7 / panther_beta-user 14 AP11.231020.013.A1 release-keys
Brave build: 1.61.94 
Chromium: 120.0.6099.56 (Official Build) (64-bit)

STEPS:

  1. Followed the steps from App exits when accessing 'send to devices' in share menu #34636 (comment)

ACTUAL RESULTS:

  • Verified that Send to devices is not shown when Sync is OFF
  • Verified that Send to devices is not shown when Sync is ON, but only one device in the sync chain
  • Verified that Send to devices is not shown when Sync is ON and more than one device is in the sync chain, but trying to share internal pages like brave://version
  • Verified that Brave is not crashing when sharing a tab via Send to devices
1 2 3
1 2 3
1 2 3

Pixel 7 (phone) / Android 14

2023-12-04_11-07-36.mp4

Galaxy Tab S8 (tablet) / Android 13

2023-12-04_11-09-40.mp4

Pixel 2 XL (phone) / Android 8

2023-12-04_11-19-12.mp4

@hffvld hffvld removed the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AlexeyBarabash AlexeyBarabash

Labels
crash feature/sync OS/Android Fixes related to Android browser functionality QA Pass - Android ARM QA Pass - Android Tab QA Pass - Android 8+ QA/Test-All-Platforms QA/Yes release-notes/exclude
Projects
None yet
Milestone
1.61.x - Release
Development

Successfully merging a pull request may close this issue.

Fixed crash accessing 'send to devices' in share menu via Sync brave/brave-core
5 participants
@kjozwiak @Uni-verse @AlexeyBarabash @brave-builds @hffvld