New issue
Advanced search Search tips

Issue 707366 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Reduce Number of JNI Calls in ClipboardAndroid

Project Member Reported by mpear...@chromium.org, Mar 31 2017

Issue description


Oh, I think I understand.  Add a new function on the Java side, GetSequenceNumber(), that's calculated correctly.  (Incremented whenever the clipboard changes.)  Give ClipboardMap a local copy of the sequence number.  Also expose a function to fetch it from the Java side.  Have the UpdateFromAndroidClipboard() first do a Java call to fetch the current sequence number.  Only if it's different does UpdateFromAndroidClipboard() bother updating its other cached variables.

And this also lets ClipboardAndroid return a plausible sequence number.  (Ask ClipboardMap to fetch the current sequence number from the Java side.)

 
Status: Started (was: Assigned)
I have a different solution to implement sequence numbers and reduce JNI calls than the one originally proposed.

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/71dfbd1f2228ae70efcea0656b3555ab539430fd

commit 71dfbd1f2228ae70efcea0656b3555ab539430fd
Author: mpearson <mpearson@chromium.org>
Date: Thu Apr 13 23:07:43 2017

Refactor Clipboard Last Modified Time Storage

This changelist is the first step to solving two problems:
* clipboard last modified time isn't remembered from run to run.  It should be.
* likewise, clipboard suggestion suppression (which happens after clearing
browsing data) should also be preserved from run to run.

The key to solving both these problems is to store the clipboard last modified
time in prefs.  (Store a 0 to represent that the clipboard shouldn't be suggested.)

To do this, we need a central place to own the last modified time value.  Upon
consultation with dcheng@, we decided this should be clipboard_android.cc.
Because Chrome on Android can be killed almost anytime, the best strategy
to have clipboard_android.cc to always have the correct value is to have
Clipboard.java "push" notification of the modification to clipboard_android.cc.

After this changelist, I will submit a changelist that stores the last_modified_time
information to prefs / reads it from prefs.

As a consequence of this decision, this change makes

* remove storage and retrieval of last modified time from Clipboard.java.
clipboard_android.cc is the canonical source now.

* clipboard_recent_content_generic.{cc,h} loses |last_modified_time_to_suppress_|, as
that suppression logic is unnecessary.  Instead, when needing to suppress a
suggestion, it tells clipboard_android to set its time to 0, which would cause
suppression.  Adds a function ClearClipboardLastModifiedTime() to clipboard.h for
this.

* remove all the logic about hashing clipboard content from Clipboard.java.  This is
not needed based on our usage of clipboard last modified time.  (This usage has been
approved by privacy.)  This also means marking a related histogram as obsolete.

* Because Clipboard.java sends clipboard changed messages now to
clipboard_android, add support for sequence numbers to clipboard_android.

* Likewise, because clipboard_android is "pushed" messages about the clipboard
being updated, it knows when its local data is out of data.  It only "pulls" clipboard
information from the Java side when someone asks it about the current state of the
clipboard and it knows that it is out of date.

* Delete ClipboardTest.java.  The only thing this tested was updating the last
modified time in Clipboard.java, which doesn't exist anymore.

TBR=sky,jif
(sky@ for the trivial changes to ui/base/test/...)
(jif@ for the refactoring / removal of logic from 	components/open_from_clipboard/...)

BUG= 682446 , 707366 

Review-Url: https://codereview.chromium.org/2812773002
Cr-Commit-Position: refs/heads/master@{#464594}

[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/components/open_from_clipboard/clipboard_recent_content_generic.cc
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/components/open_from_clipboard/clipboard_recent_content_generic.h
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/components/open_from_clipboard/clipboard_recent_content_generic_unittest.cc
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/tools/metrics/histograms/histograms.xml
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/ui/android/BUILD.gn
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/ui/android/java/src/org/chromium/ui/base/Clipboard.java
[delete] https://crrev.com/94182b78878d55cb968e15312c66abfe21e3caf4/ui/android/junit/src/org/chromium/ui/base/ClipboardTest.java
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/ui/base/android/ui_base_jni_registrar.cc
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/ui/base/clipboard/clipboard.cc
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/ui/base/clipboard/clipboard.h
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/ui/base/clipboard/clipboard_android.cc
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/ui/base/clipboard/clipboard_android.h
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/ui/base/test/test_clipboard.cc
[modify] https://crrev.com/71dfbd1f2228ae70efcea0656b3555ab539430fd/ui/base/test/test_clipboard.h

Status: Fixed (was: Started)
Reduced!

I don't see any way to reduce the number further.

Reduced!

I don't see any way to reduce the number further.

Sign in to add a comment