Disable Reader mode in fullscreen mode or when infobars are shown

Disable Reader mode in fullscreen mode or when infobars are shown

BUG=495560, 495808
Committed: https://crrev.com/901a3e766236ed08252799f3a5737f65568c4a49
Cr-Commit-Position: refs/heads/master@{#333164}

[aruslan, 2015-06-05 02:49:28]

aruslan@chromium.org changed reviewers:
+ dtrainor@chromium.org

[aruslan, 2015-06-05 02:49:29]

David PTAL thanks!

[aruslan, 2015-06-05 02:49:37]

The CQ bit was checked by aruslan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-06-05 02:50:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1152013007/1

[commit-bot: I haz the power, 2015-06-05 03:22:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-05 03:22:35]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2015-06-05 05:33:05]

https://codereview.chromium.org/1152013007/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/Tab.java (right):

https://codereview.chromium.org/1152013007/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/Tab.java:2407: public void
onEnterFullscreen() {
Let's pipe fullscreen state through the LayoutManager and onto the reader mode
component.  I think that flow might already exist there.

[aruslan, 2015-06-05 20:14:47]

aruslan@chromium.org changed reviewers:
+ tedchoc@chromium.org

[aruslan, 2015-06-05 20:14:48]

Ted, PTAL re fullscreen thanks!

[aruslan, 2015-06-05 20:41:51]

The CQ bit was checked by aruslan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-06-05 20:43:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1152013007/20001

[commit-bot: I haz the power, 2015-06-05 21:38:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-05 21:38:08]

Dry run: This issue passed the CQ dry run.

[Ted C, 2015-06-05 21:58:29]

https://chromiumcodereview.appspot.com/1152013007/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarContainer.java
(right):

https://chromiumcodereview.appspot.com/1152013007/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarContainer.java:363:
observer.onRemoveInfoBar(this,  infoBar, mInfoBars.isEmpty());
you have one too many spaces after "this,"

https://chromiumcodereview.appspot.com/1152013007/diff/20001/chrome/android/j...
File
chrome/android/java_staging/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java
(right):

https://chromiumcodereview.appspot.com/1152013007/diff/20001/chrome/android/j...
chrome/android/java_staging/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java:141:
mTab.removeObserver(this);
not strictly necessary since onDestroy came from the Tab, so it will drop all of
its observers when it's dead

https://chromiumcodereview.appspot.com/1152013007/diff/20001/chrome/android/j...
chrome/android/java_staging/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java:166:
if (mReaderModePanel == null) return;
When is the readermodepanel created?  Just wondering if it could end up
happening after entering fullscreen or showing an info bar such that you miss
the signals you expect.

[aruslan, 2015-06-05 22:05:34]

aruslan@chromium.org changed reviewers:
- dtrainor@chromium.org

[aruslan, 2015-06-05 22:05:35]

Thanks, Ted PTAL.

https://chromiumcodereview.appspot.com/1152013007/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarContainer.java
(right):

https://chromiumcodereview.appspot.com/1152013007/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/infobar/InfoBarContainer.java:363:
observer.onRemoveInfoBar(this,  infoBar, mInfoBars.isEmpty());
On 2015/06/05 21:58:29, Ted C wrote:
> you have one too many spaces after "this,"

Done.

https://chromiumcodereview.appspot.com/1152013007/diff/20001/chrome/android/j...
File
chrome/android/java_staging/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java
(right):

https://chromiumcodereview.appspot.com/1152013007/diff/20001/chrome/android/j...
chrome/android/java_staging/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java:141:
mTab.removeObserver(this);
On 2015/06/05 21:58:29, Ted C wrote:
> not strictly necessary since onDestroy came from the Tab, so it will drop all
of
> its observers when it's dead

Done, I didn't have it originally precisely for that reason.
I liked it better with an explicit removeObserver, though :)

https://chromiumcodereview.appspot.com/1152013007/diff/20001/chrome/android/j...
chrome/android/java_staging/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java:166:
if (mReaderModePanel == null) return;
On 2015/06/05 21:58:29, Ted C wrote:
> When is the readermodepanel created?  Just wondering if it could end up
> happening after entering fullscreen or showing an info bar such that you miss
> the signals you expect.

Its lifetime is the same as the ReaderModeManager's; it's created conditionally
in the ReaderModeManager constructor if the feature is enabled.

[Ted C, 2015-06-05 22:08:20]

lgtm

[aruslan, 2015-06-05 22:10:55]

The CQ bit was checked by aruslan@chromium.org

[commit-bot: I haz the power, 2015-06-05 22:11:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1152013007/40001

[commit-bot: I haz the power, 2015-06-05 22:44:18]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-05 22:45:55]

Patchset 3 (id:??) landed as
https://crrev.com/901a3e766236ed08252799f3a5737f65568c4a49
Cr-Commit-Position: refs/heads/master@{#333164}