New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 637988 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Traveling - Back 2/6
Closed: Sep 2016
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 597756



Sign in to add a comment

Build feedback mechanism for capturing client logs

Project Member Reported by amineer@chromium.org, Aug 15 2016

Issue description

Over to dtrainor@ to figure out how this should be scoped.
 
Blocking: 597756
Owner: nyquist@chromium.org
Status: Started (was: Assigned)
CLs in review:
Framework: https://codereview.chromium.org/2362693003/
BlimpContents Visibility: https://codereview.chromium.org/2371503002/
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 27 2016

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

commit d98c5c13457fae21894b7413b7407eb0e74d76e1
Author: nyquist <nyquist@chromium.org>
Date: Tue Sep 27 07:17:37 2016

Add framework for sending feedback for Blimp.

Before this CL, there was never any information about Blimp included in
the feedback data provided to HelpAndFeedback. This CL adds a framework
for adding such feedback data. The only field that is currently set
is whether Blimp is supported or not, and it is not there unless Blimp
is supported.

The feedback collection is cross-platform, but it is currently only
used for Android, and initiated from Java in the FeedbackCollector.

The data needs to end up as a Bundle with only <String, String> data,
so the cross platform code also uses a map of strings as keys and
values.

The JNI-bridge code takes the source map and converts it into a Java
object holding a Map<String, String> which is provided to the
FeedbackCollector. There is no additional business logic in Java.
Since there is no way to pass for example std::unordered_map from C++
to Java, the keys and values are passed as simple Java-arrays
during construction of the Java object instead. After the Java
object has been constructed, it lives on its own and does not require
access to functions in C++.

BUG= 637988 

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

[modify] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/app/android/javatests/src/org/chromium/blimp/core/MockBlimpClientContext.java
[modify] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/BUILD.gn
[modify] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/android/blimp_client_context_impl_android.cc
[modify] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/android/blimp_client_context_impl_android.h
[modify] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/android/java/src/org/chromium/blimp/core/BlimpClientContextImpl.java
[modify] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/android/java/src/org/chromium/blimp/core/DummyBlimpClientContext.java
[modify] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/blimp_client_context_impl.cc
[modify] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/blimp_client_context_impl.h
[add] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/feedback/BUILD.gn
[add] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/feedback/android/blimp_feedback_data_android.cc
[add] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/feedback/android/blimp_feedback_data_android.h
[add] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/feedback/android/java/src/org/chromium/blimp/core/feedback/BlimpFeedbackData.java
[add] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/feedback/blimp_feedback_data.cc
[add] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/feedback/blimp_feedback_data.h
[add] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/core/feedback/blimp_feedback_data_unittest.cc
[modify] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/blimp/client/public/android/java/src/org/chromium/blimp_public/BlimpClientContext.java
[modify] https://crrev.com/d98c5c13457fae21894b7413b7407eb0e74d76e1/chrome/android/java/src/org/chromium/chrome/browser/feedback/FeedbackCollector.java

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 27 2016

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

commit 9e08bbc902fb4cda99334956aa1eca8bfe79f65c
Author: nyquist <nyquist@chromium.org>
Date: Tue Sep 27 07:59:10 2016

Add Blimp feedback data about visible BlimpContents.

Whether a BlimpContents is visible or not is an important concept for
Blimp feedback. This CL adds support for emitting data about whether
any BlimpContents is visible.

To be able to do this, we now expose the list of all active
BlimpContents from the BlimpContentsManager, and the
BlimpCompositorManager now exposes whether it is visible or not.
The state can still be controlled directly from BlimpContentsImpl
by invoking Show()/Hide(), which is what the test code is invoking.

Since this CL adds another test for BlimpContentsManager, it also
cleans up that test file by making the test-class
BlimpContentsManagerTest set up all the common parts of all the tests.

BUG= 637988 

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

[modify] https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c/blimp/client/core/blimp_client_context_impl.cc
[modify] https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c/blimp/client/core/compositor/blimp_compositor_manager.h
[modify] https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c/blimp/client/core/contents/blimp_contents_manager.cc
[modify] https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c/blimp/client/core/contents/blimp_contents_manager.h
[modify] https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c/blimp/client/core/contents/blimp_contents_manager_unittest.cc
[modify] https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c/blimp/client/core/feedback/BUILD.gn
[modify] https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c/blimp/client/core/feedback/blimp_feedback_data.cc
[modify] https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c/blimp/client/core/feedback/blimp_feedback_data.h
[modify] https://crrev.com/9e08bbc902fb4cda99334956aa1eca8bfe79f65c/blimp/client/core/feedback/blimp_feedback_data_unittest.cc

Status: Fixed (was: Started)
Marking this as fixed, but still need to fix issue 650819 to get this fully working.
Labels: Archive-Blimp

Sign in to add a comment