Retrieve SupportedProfileType when restoring from persistent/saved state [chromium/src : main]
Shu Yang (Gerrit)
Shu Yang added 1 comment![]( "Open in Gerrit")
}Since there is no else clause, mSupportedProfileType may still be uninitialized.
Just want to make sure this is intended.
Related details
- Charles Hager
- Michael Wu
- Sirisha Kavuluru
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Charles Hager (Gerrit)
Charles Hager added 1 comment![]( "Open in Gerrit")
Since there is no else clause, mSupportedProfileType may still be uninitialized.
Just want to make sure this is intended.
I think we probably want the profile type to always be initialized.....what should be the fallback here? When might the multi-instance manager be null?
Maybe we should add a fallback and also emit a histogram to keep tabs on how common (or rare) this is?
Related details
- Michael Wu
- Sirisha Kavuluru
Michael Wu (Gerrit)
Michael Wu added 1 comment![]( "Open in Gerrit")
Charles HagerSince there is no else clause, mSupportedProfileType may still be uninitialized.
Just want to make sure this is intended.
I think we probably want the profile type to always be initialized.....what should be the fallback here? When might the multi-instance manager be null?
Maybe we should add a fallback and also emit a histogram to keep tabs on how common (or rare) this is?
- Charles Hager
- Shu Yang
- Sirisha Kavuluru
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Charles Hager (Gerrit)
Charles Hager added 2 comments![]( "Open in Gerrit")
public static @SupportedProfileType int readProfileType(int instanceId) {Nit: JavaDocs
Related details
- Michael Wu
- Shu Yang
- Sirisha Kavuluru
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Shu Yang (Gerrit)
Shu Yang added 1 comment![]( "Open in Gerrit")
mSupportedProfileType = SupportedProfileType.MIXED;From non-owner perspective:
Comparing with the original CL:
https://chromium-review.googlesource.com/c/chromium/src/+/7208039/10/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
The else clause used to has mWindowId == 0, which is a valid window id.
And we used to follow the normal procedure to determine mSupportedProfileType (based on FF/intent/ChromeSharedPreferences). Now we force it to SupportedProfileType.MIXED, which might be a behavior change.
But whether this is Okay depends on when we will get into the else clause.
Related details
- Michael Wu
- Sirisha Kavuluru
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wu (Gerrit)
Michael Wu added 2 comments![]( "Open in Gerrit")
From non-owner perspective:
Comparing with the original CL:
https://chromium-review.googlesource.com/c/chromium/src/+/7208039/10/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
The else clause used to has mWindowId == 0, which is a valid window id.
And we used to follow the normal procedure to determine mSupportedProfileType (based on FF/intent/ChromeSharedPreferences). Now we force it to SupportedProfileType.MIXED, which might be a behavior change.
But whether this is Okay depends on when we will get into the else clause.
Since we don't know when the else clause occurs, using the normal procedure to determine supportedProfileType makes sense to me.
public static @SupportedProfileType int readProfileType(int instanceId) {Michael WuNit: JavaDocs
Done
Related details
- Sirisha Kavuluru
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |